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

Wrong swap fee is applied under certain circumstances

valya

Root + Impact

Description

  • Pools, where ReFi is currency1 and SwapParams::zeroForOne is true: ReFiSwapRebateHook::_isReFiBuy will return false and ReFiSwapRebateHook::_beforeSwap will emit ReFiSold event even if ReFi was bought and sellFee will be applied instead of buyFee.

  • Pools, where ReFi is currency0 and SwapParams::zeroForOne is false: ReFiSwapRebateHook::_isReFiBuy will return true and ReFiBought will emit but the ReFi was sold and buyFee is applied instead of sellFee.

function _isReFiBuy(PoolKey calldata key, bool zeroForOne) internal view returns (bool) {
bool IsReFiCurrency0 = Currency.unwrap(key.currency0) == ReFi;
​
@> if (IsReFiCurrency0) {
@> return zeroForOne;
} else {
@> return !zeroForOne;
}
}

Risk

Likelihood:

  • The issue occurs in both cases:

    1. when the ReFi token is currency1 in the pool and zeroForOne is true in SwapParams.

    2. when the ReFi token is currency0 in the pool and zeroForOne is false in SwapParams.

Impact:

  • The wrong fee is applied during swap

Proof of Concept

Add the following test and HookWrapper contract to RebateFiHookTest.t.sol

contract HookWrapper is ReFiSwapRebateHook {
constructor(IPoolManager _poolManager, address _ReFi) ReFiSwapRebateHook(_poolManager, _ReFi) {}
​
function isReFiBuy(PoolKey calldata key, bool zeroForOne) external view returns (bool) {
return _isReFiBuy(key, zeroForOne);
}
}
​
function test_isReFiBuy() public {
// deploy hook wrapper
bytes memory creationCode1 = type(HookWrapper).creationCode;
bytes memory constructorArgs1 = abi.encode(manager, address(reFiToken));
uint160 flags1 = uint160(Hooks.BEFORE_INITIALIZE_FLAG | Hooks.AFTER_INITIALIZE_FLAG | Hooks.BEFORE_SWAP_FLAG);
(, bytes32 salt1) = HookMiner.find(address(this), flags1, creationCode1, constructorArgs1);
hookWrapper = new HookWrapper{salt: salt1}(manager, address(reFiToken));
​
// ReFi is currency1 and zeroForOne = true -> ReFi should be isReFiBuy, but it is not
Currency someCurrency = Currency.wrap(address(5));
(key,) = initPool(someCurrency, reFiCurrency, rebateHook, LPFeeLibrary.DYNAMIC_FEE_FLAG, SQRT_PRICE_1_1_s);
bool zeroForOne = true;
bool isRefiBuy = hookWrapper.isReFiBuy(key, zeroForOne);
assertEq(isRefiBuy, false);
​
// ReFi is currency0 and zeroForOne = false -> ReFi should be isReFiBuy, but it is not
Currency someCurrency2 = Currency.wrap(0x212224d2F2d262cd093EE13240cA4873FccbbA3d);
(key,) = initPool(reFiCurrency, someCurrency2, rebateHook, LPFeeLibrary.DYNAMIC_FEE_FLAG, SQRT_PRICE_1_1_s);
zeroForOne = false;
isRefiBuy = hookWrapper.isReFiBuy(key, zeroForOne);
assertEq(isRefiBuy, false);
}
​

Recommended Mitigation

Add the following changes to the code:

​
function _isReFiBuy(PoolKey calldata key, bool zeroForOne) internal view returns (bool) {
bool IsReFiCurrency0 = Currency.unwrap(key.currency0) == ReFi;
​
- if (IsReFiCurrency0) {
- return zeroForOne;
- } else {
- return !zeroForOne;
- }
​
+ if (IsReFiCurrency0 && (zeroForOne == false)) {
+ return true;
+ }
+ if (!IsReFiCurrency0 && (zeroForOne == true)) {
+ return true;
+ }
+ return false;
}
Updates

Lead Judging Commences

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

Inverted buy/sell logic when ReFi is currency0, leading to incorrect fee application.

Support

FAQs

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

Give us feedback!