RebateFi Hook
First Flight #53
Beginner FriendlyDeFi
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

Incorrect Token Validation in _beforeInitialize Causes Missing ReFi Token Enforcement

shashankwcw

Root + Impact

The expected behavior is that the hook validates whether the ReFi token is present in the pool before initialization. The pool uses currency0 and currency1 to represent its paired assets. Proper validation requires checking both currencies so that initialization cannot proceed unless either currency0 or currency1 equals the ReFi token.

Description

Initialization should only succeed when at least one of the pool’s currencies (currency0 or currency1) matches the ReFi token, ensuring protocol constraints are enforced.

  • currency0 is completely ignored, allowing pools that exclude ReFi in currency0 to be initialized without restriction.

The use of && with the same condition results in no effective validation of alternative valid scenarios where ReFi may be currency0.

// Root cause in the codebase with @> marks to highlight the relevant section
function _beforeInitialize(address, PoolKey calldata key, uint160) internal view override returns (bytes4) {
@> if (Currency.unwrap(key.currency1) != ReFi &&
@> Currency.unwrap(key.currency1) != ReFi) {
revert ReFiNotInPool();
}
return BaseHook.beforeInitialize.selector;
}In this

Risk

Likelihood:

  • Occurs whenever a pool is initialized where the ReFi token is intended to be enforced as either currency0 or currency1, but ReFi is placed in currency0.

Impact:

  • Unauthorized or non-compliant pools can be initialized.

Protocol invariants relying on ReFi presence are broken, potentially enabling misconfigured liquidity pools and downstream accounting or reward distribution errors.

Proof of Concept

  • In the test output we can clearly see that the test fails because the code only checks if currency 1 is the refi token or not.

function test_PoC_beforeInitialize_reverts_when_ReFi_is_currency0() public {
// 1) fresh manager/routers so we don't interfere with setUp state
deployFreshManagerAndRouters();
​
// 2) create two ERC20s; we need address(refi) < address(other)
MockERC20 other = new MockERC20("OTHER", "OTH", 18);
MockERC20 refi = new MockERC20("RE-FI", "RFI", 18);
​
Currency cOther = Currency.wrap(address(other));
Currency cRefi = Currency.wrap(address(refi));
​
// 3) if ordering is not correct, redeploy 'other' up to some attempts until address(refi) < address(other)
// (This is a pragmatic approach for test environments where addresses are sequential-ish.)
uint256 attempts = 0;
while (address(refi) >= address(other) && attempts < 50) {
other = new MockERC20("OTHER", "OTH", 18);
cOther = Currency.wrap(address(other));
attempts++;
}
require(address(refi) < address(other), "Could not make refi address < other address; retry test");
​
// 4) deploy a hook for this ReFi token using HookMiner (so PoolManager accepts the hook address)
bytes memory creationCode = type(ReFiSwapRebateHook).creationCode;
bytes memory constructorArgs = abi.encode(manager, address(refi));
​
uint160 flags = uint160(
Hooks.BEFORE_INITIALIZE_FLAG |
Hooks.AFTER_INITIALIZE_FLAG |
Hooks.BEFORE_SWAP_FLAG
);
​
(address expectedHook, bytes32 salt) = HookMiner.find(
address(this),
flags,
creationCode,
constructorArgs
);
​
// deploy using the mined salt (CREATE2)
ReFiSwapRebateHook hook = new ReFiSwapRebateHook{salt: salt}(manager, address(refi));
require(address(hook) == expectedHook, "Hook address mismatch (mined)");
​
// 5) Now call initPool with ReFi as currency0 (address(refi) < address(other) guarantees canonical order)
// and expect the buggy hook's beforeInitialize to revert with ReFiNotInPool
bytes memory expectedErr = abi.encodeWithSelector(ReFiSwapRebateHook.ReFiNotInPool.selector);
vm.expectRevert(expectedErr);
​
// this should hit the hook's beforeInitialize callback and (if buggy) revert ReFiNotInPool()
initPool(cRefi, cOther, hook, LPFeeLibrary.DYNAMIC_FEE_FLAG, SQRT_PRICE_1_1_s);
​
// If the test reaches here without revert, the hook did NOT revert — this means the hook is likely fixed.
// Note: the vm.expectRevert() will cause the test to fail if no revert occurred.
}
​
​
​
/*
///////////////////////////////////////////// Output /////////////////////////////////////
*/
​
[68231765] TestReFiSwapRebateHook::test_PoC_beforeInitialize_reverts_when_ReFi_is_currency0()
├─ [6957545] → new PoolManager@0xD16d567549A2a2a2005aEACf7fB193851603dd70
│ ├─ emit OwnershipTransferred(previousOwner: 0x0000000000000000000000000000000000000000, newOwner: TestReFiSwapRebateHook: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496])
│ └─ ← [Return] 34623 bytes of code
├─ [2064771] → new PoolSwapTest@0x96d3F6c20EEd2697647F543fE6C08bC2Fbf39758
│ └─ ← [Return] 10310 bytes of code
├─ [1101289] → new SwapRouterNoChecks@0x13aa49bAc059d709dd0a18D6bb63290076a702D7
│ └─ ← [Return] 5498 bytes of code
├─ [1846957] → new PoolModifyLiquidityTest@0xDB25A7b768311dE128BBDa7B8426c3f9C74f3240
│ └─ ← [Return] 9222 bytes of code
├─ [1375580] → new PoolModifyLiquidityTestNoChecks@0x3381cD18e2Fb4dB236BF0525938AB6E43Db0440f
│ └─ ← [Return] 6868 bytes of code
├─ [1550396] → new PoolDonateTest@0x756e0562323ADcDA4430d6cb456d9151f605290B
│ └─ ← [Return] 7741 bytes of code
├─ [1296683] → new PoolTakeTest@0x1aF7f588A501EA2B5bB3feeFA744892aA2CF00e6
│ └─ ← [Return] 6474 bytes of code
├─ [924111] → new PoolClaimsTest@0xe8dc788818033232EF9772CB2e6622F1Ec8bc840
│ └─ ← [Return] 4613 bytes of code
├─ [6353741] → new PoolNestedActionsTest@0x3Cff5E7eBecb676c3Cb602D0ef2d46710b88854E
│ ├─ [4514755] → new NestedActionExecutor@0x04cf47d237A7e4358C874a9767f79bBFC62F8495
│ │ └─ ← [Return] 21188 bytes of code
│ └─ ← [Return] 8546 bytes of code
├─ [0] VM::addr(<pk>) [staticcall]
│ └─ ← [Return] feeController: [0xb52F5153576Ca8b1d3eD645A8F76809Bd1a62620]
├─ [0] VM::label(feeController: [0xb52F5153576Ca8b1d3eD645A8F76809Bd1a62620], "feeController")
│ └─ ← [Return]
├─ [3003679] → new ActionsRouter@0x27cc01A4676C73fe8b6d0933Ac991BfF1D77C4da
│ └─ ← [Return] 14777 bytes of code
├─ [23988] PoolManager::setProtocolFeeController(feeController: [0xb52F5153576Ca8b1d3eD645A8F76809Bd1a62620])
│ ├─ emit ProtocolFeeControllerUpdated(protocolFeeController: feeController: [0xb52F5153576Ca8b1d3eD645A8F76809Bd1a62620])
│ └─ ← [Stop]
├─ [1193737] → new MockERC20@0x796f2974e3C1af763252512dd6d521E9E984726C
│ └─ ← [Return] 5720 bytes of code
├─ [1193737] → new MockERC20@0x92a6649Fdcc044DA968d94202465578a9371C7b1
│ └─ ← [Return] 5720 bytes of code
├─ [1193737] → new MockERC20@0xDA5A5ADC64C8013d334A0DA9e711B364Af7A4C2d
│ └─ ← [Return] 5720 bytes of code
├─ [1736687] → new ReFiSwapRebateHook@0x91D6dF936455e866cA64f1d4297A25d5ED997080
│ ├─ emit OwnershipTransferred(previousOwner: 0x0000000000000000000000000000000000000000, newOwner: TestReFiSwapRebateHook: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496])
│ └─ ← [Return] 8533 bytes of code
├─ [0] VM::expectRevert(custom error 0xf28dceb3: 000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000047ac0d4d700000000000000000000000000000000000000000000000000000000)
│ └─ ← [Return]
├─ [7547] PoolManager::initialize(PoolKey({ currency0: 0x92a6649Fdcc044DA968d94202465578a9371C7b1, currency1: 0xDA5A5ADC64C8013d334A0DA9e711B364Af7A4C2d, fee: 8388608 [8.388e6], tickSpacing: 60, hooks: 0x91D6dF936455e866cA64f1d4297A25d5ED997080 }), 79228162514264337593543950336 [7.922e28])
│ ├─ [1598] ReFiSwapRebateHook::beforeInitialize(TestReFiSwapRebateHook: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496], PoolKey({ currency0: 0x92a6649Fdcc044DA968d94202465578a9371C7b1, currency1: 0xDA5A5ADC64C8013d334A0DA9e711B364Af7A4C2d, fee: 8388608 [8.388e6], tickSpacing: 60, hooks: 0x91D6dF936455e866cA64f1d4297A25d5ED997080 }), 79228162514264337593543950336 [7.922e28])
│ │ └─ ← [Revert] ReFiNotInPool()

Recommended Mitigation

  • Just use || (or) operator instead of && (and)

  • Don't compare currency1 only also compare currency0

- if (Currency.unwrap(key.currency1) != ReFi &&
- Currency.unwrap(key.currency1) != ReFi) {
+ if (Currency.unwrap(key.currency1) != ReFi ||
+ Currency.unwrap(key.currency0) != ReFi) {
Updates

Lead Judging Commences

chaossr Lead Judge
15 days ago
chaossr Lead Judge 12 days ago
Submission Judgement Published
Validated
Assigned finding tags:

Faulty pool check; only checks currency1 twice, omitting currency0.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!