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

Incorrect token validation logic in _beforeInitialize()

cryptomv3

Incorrect token validation logic in ReFiSwapRebateHook::_beforeInitialize(), resulting in a restriction on pool creation.


Description

  • The ReFiSwapRebateHook::_beforeInitialize() should validate that ReFi token is in the pool (in key.currency0 or in key.currency1) before initialization

  • But this only checks the key.currency1 position

// 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(); // @audit M-1 duplicate checking the same currency (currency1)
}
​
return BaseHook.beforeInitialize.selector;
}

Risk

Likelihood:

  • This will occur when a user attempts to create a pool with an incorrect token order.

Impact:

  • The user cannot initialize the pool with an incorrect token order;

  • It is possible to break UX or the creation of a pool with an incorrect configuration may be blocked.

Proof of Concept

Since in the ReFiSwapRebateHook::_beforeInitialize() we check only currency1 then:

  • First pool inititialization reverts because key.currency1 != ReFi (wrong order)

  • Second pool inititialization will not revert, because key.currency1 == ReFi (correct order)

Therefore, the pool can be initialized only when key.currency1 == ReFi.

function test_PoC_IncorrectTokenCheckInBeforeInitialize() public {
// In the ReFiSwapRebateHook::_beforeInitialize() we check only currency1:
// "Currency.unwrap(key.currency1) != ReFi && Currency.unwrap(key.currency1) != ReFi" then:
// This reverts because key.currency1 != ReFi (wrong order)
vm.expectRevert();
(PoolKey memory wrongKey,) =
initPool(reFiCurrency, tokenCurrency, rebateHook, LPFeeLibrary.DYNAMIC_FEE_FLAG, SQRT_PRICE_1_1_s);
​
// This will not revert, because key.currency1 == ReFi (correct order)
(PoolKey memory rightKey,) =
initPool(tokenCurrency, reFiCurrency, rebateHook, LPFeeLibrary.DYNAMIC_FEE_FLAG, SQRT_PRICE_1_1_s);
​
console.log("Therefore, the pool can be initialized only when key.currency1 == ReFi");
}

Recommended Mitigation

Add a check for key.currency0 and key.currency1 in the ReFiSwapRebateHook::_beforeInitialize() function.

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(); // @audit M-1 duplicate checking the same currency (currency1)
}
​
return BaseHook.beforeInitialize.selector;
}
Updates

Lead Judging Commences

chaossr Lead Judge 11 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!