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

`RebateFiHook::_beforeInitialize` only checks currency1, blocking valid pools where ReFi is currency0

themilenkov

`RebateFiHook_beforeInitialize` compares key.currency1 to ReFi twice and never checks key.currency0, effectively enforcing “ReFi must be currency1”, initialization will revert whenever address ordering makes ReFi the currency0 of the pair

Description

  • **Normal behavior: **
    Before pool initialization, the hook should verify that the ReFi token is part of the pair, regardless of whether it sits in currency0 or currency1.

  • Issue:

    The code mistakenly checks currency1 twice, never considering currency0. When ReFi sorts to currency0 (common, depending on address ordering), initialization reverts with ReFiNotInPool(), blocking pool creation and all downstream operations for that pair.

// Root cause in code (typo: checks currency1 twice)
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;
}

Risk

Likelihood:

  • Uniswap v4 uses deterministic currency ordering; for many pairs ReFi will naturally end up as currency0 (e.g., when paired with a token deployed after ReFi). In those cases, initialization always hits this path.


Impact:

  • Legitimate pools (ReFi as currency0) cannot be initialized; liquidity cannot be added; swaps are impossible for those pairs.

Proof of Concept

Add the following test to RebateFiHookTest.t.sol. It deploys a new token after ReFi so its address is higher, making ReFi the currency0 for that pair. With the current bug, _beforeInitialize reverts with ReFiNotInPool().

function test_BeforeInitialize_Reverts_When_ReFi_Is_Currency0() public {
// Deploy a new token AFTER reFiToken so its address is greater than ReFi's,
// which makes ReFi sort to currency0 vs this new token.
MockERC20 laterToken = new MockERC20("LATER", "LTR", 18);
Currency laterCurrency = Currency.wrap(address(laterToken));
​
// Expect the hook to revert because it only checks currency1 (bug)
vm.expectRevert(ReFiSwapRebateHook.ReFiNotInPool.selector);
​
// Try to initialize a new pool where ReFi will be currency0
// (dynamic-fee flag required by the hook's afterInitialize)
initPool(
reFiCurrency, // will become currency0 vs laterCurrency
laterCurrency, // will become currency1
rebateHook,
LPFeeLibrary.DYNAMIC_FEE_FLAG,
SQRT_PRICE_1_1_s
);
}

Recommended Mitigation

Check both sides for membership:

function _beforeInitialize(address, PoolKey calldata key, uint160)
internal view override returns (bytes4)
{
- if (Currency.unwrap(key.currency1) != ReFi &&
- Currency.unwrap(key.currency1) != ReFi) {
+ if (
+ Currency.unwrap(key.currency0) != ReFi &&
+ Currency.unwrap(key.currency1) != ReFi
+ ) {
revert ReFiNotInPool();
}
​
return BaseHook.beforeInitialize.selector;
}
Updates

Lead Judging Commences

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!