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

RebateFi Hook - 4 Critical, 3 High Severity Findings

vyqno

Pool Initialization Rejects Valid ReFi Pools

Description

Pools should be created when ReFi token is either currency0 or currency1. The validation logic checks currency1 twice and never checks currency0, causing all pools where ReFi has the lower address to revert.

function beforeInitialize(
address,
PoolKey calldata key,
uint160
) external override onlyPoolManager returns (bytes4) {
// @> if (Currency.unwrap(key.currency1) != ReFi &&
// @> Currency.unwrap(key.currency1) != ReFi) {
revert("Pool must include ReFi token");
}
return this.beforeInitialize.selector;
}

Risk

Likelihood:

  • Uniswap V4 sorts currencies by address - lower address becomes currency0

  • Every pool deployment where ReFi address < paired token address triggers this bug

Impact:

  • ~50% of valid pool configurations rejected at initialization

  • Protocol unusable for token pairs where ReFi happens to have lower address

Proof of Concept

When ReFi (0x1111) pairs with USDC (0x2222), Uniswap sorts them as currency0=ReFi, currency1=USDC. The check evaluates currency1 != ReFi which is 0x2222 != 0x1111 = true, so it reverts despite ReFi being present.

function test_initBlocksValidPool() public {
PoolKey memory key = PoolKey({
currency0: Currency.wrap(address(reFi)), // 0x1111 (lower)
currency1: Currency.wrap(address(usdc)), // 0x2222 (higher)
fee: 3000,
tickSpacing: 60,
hooks: hook
});
vm.expectRevert("Pool must include ReFi token");
poolManager.initialize(key, SQRT_PRICE_1_1);
}

Recommended Mitigation

- if (Currency.unwrap(key.currency1) != ReFi &&
- Currency.unwrap(key.currency1) != ReFi) {
+ if (Currency.unwrap(key.currency0) != ReFi &&
+ Currency.unwrap(key.currency1) != ReFi) {
revert("Pool must include ReFi token");
}

Buy/Sell Fee Logic Returns Inverted Values

Description

Buyers should pay 0% fee and sellers should pay 0.3% fee. The _isReFiBuy function returns true when users sell and false when users buy, causing the opposite fees to be applied.

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:

  • Every swap calls this function to determine fee

  • The return values are backwards for all four possible scenarios

Impact:

  • Buyers charged 0.3% when they should pay nothing

  • Sellers pay nothing when they should pay 0.3%

  • Protocol revenue model completely inverted

Proof of Concept

When ReFi is currency0 and user sells ReFi (zeroForOne=true), the function returns true indicating a "buy". When user buys ReFi (zeroForOne=false), it returns false indicating a "sell". Both cases apply the wrong fee.

function test_feeLogicInverted() public {
// ReFi is currency0
// User SELLS ReFi: zeroForOne=true, returns true (claims BUY), applies 0%
// User BUYS ReFi: zeroForOne=false, returns false (claims SELL), applies 0.3%
bool sellResult = hook.exposed_isReFiBuy(key, true);
bool buyResult = hook.exposed_isReFiBuy(key, false);
assertEq(sellResult, true); // Wrong - selling returns "buy"
assertEq(buyResult, false); // Wrong - buying returns "sell"
}

Recommended Mitigation

if (IsReFiCurrency0) {
- return zeroForOne;
+ return !zeroForOne;
} else {
- return !zeroForOne;
+ return zeroForOne;
}

Token Transfer Silently Fails Without Return Check

Description

The withdrawTokens function should revert when transfer fails. Non-standard ERC20 tokens like USDT return false on failure instead of reverting, but the return value is ignored and the success event emits regardless.

function withdrawTokens(address token, address to, uint256 amount) external onlyOwner {
// @> IERC20(token).transfer(to, amount);
emit TokensWithdrawn(to, token, amount);
}

Risk

Likelihood:

  • Occurs when withdrawing non-standard ERC20s (USDT, BNB, OMG)

  • Triggers when hook has insufficient balance or token is paused

Impact:

  • Tokens remain stuck in contract while event claims successful withdrawal

  • Off-chain systems show incorrect balances

Proof of Concept

When hook has 500 tokens and owner withdraws 1000, the non-standard token returns false. The return value is not checked so execution continues and emits TokensWithdrawn(1000) while actual balances remain unchanged.

function test_silentTransferFailure() public {
weirdToken.mint(address(hook), 500e18);
hook.withdrawTokens(address(weirdToken), owner, 1000e18);
assertEq(weirdToken.balanceOf(address(hook)), 500e18); // unchanged
assertEq(weirdToken.balanceOf(owner), 0); // got nothing
}

Recommended Mitigation

+ import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
+ using SafeERC20 for IERC20;
​
function withdrawTokens(address token, address to, uint256 amount) external onlyOwner {
- IERC20(token).transfer(to, amount);
+ IERC20(token).safeTransfer(to, amount);
emit TokensWithdrawn(token, to, amount);
}

Event Emits Parameters in Wrong Order

Description

The TokensWithdrawn event declares parameters as (token, to, amount) but emits them as (to, token, amount). Indexed parameters are stored in swapped positions permanently on-chain.

// @> event TokensWithdrawn(address indexed token, address indexed to, uint256 amount);
​
function withdrawTokens(address token, address to, uint256 amount) external onlyOwner {
IERC20(token).transfer(to, amount);
// @> emit TokensWithdrawn(to, token, amount);
}

Risk

Likelihood:

  • Every withdrawal emits event with swapped parameters

  • Indexed data stored permanently in transaction logs

Impact:

  • Queries filtering by token address return zero results

  • Off-chain indexers and subgraphs display incorrect data

Proof of Concept

When withdrawing token 0x1111 to recipient 0x2222, the event stores 0x2222 in topic[1] (token slot) and 0x1111 in topic[2] (recipient slot). Any query for "withdrawals of token 0x1111" finds nothing.

function test_eventParamsSwapped() public {
vm.recordLogs();
hook.withdrawTokens(address(0x1111), address(0x2222), 1000e18);
Vm.Log[] memory logs = vm.getRecordedLogs();
// topic[1] contains 0x2222 (recipient) instead of token
// topic[2] contains 0x1111 (token) instead of recipient
assertEq(logs[0].topics[1], bytes32(uint256(uint160(0x2222))));
}

Recommended Mitigation

- emit TokensWithdrawn(to, token, amount);
+ emit TokensWithdrawn(token, to, amount);

Owner Can Set Unlimited Fee Enabling Rug Pull

Description

Fee changes should be bounded to prevent malicious values. The ChangeFee function accepts any uint24 value up to 16,777,215 without validation, allowing fees that exceed swap amounts.

function ChangeFee(
bool _isBuyFee,
uint24 _buyFee,
bool _isSellFee,
uint24 _sellFee
) external onlyOwner {
// @> if(_isBuyFee) buyFee = _buyFee;
// @> if(_isSellFee) sellFee = _sellFee;
}

Risk

Likelihood:

  • Malicious or compromised owner calls ChangeFee with max value

  • No timelock allows instant execution

Impact:

  • Fee exceeding swap amount causes all swaps to revert

  • User funds trapped in pool with no way to exit

Proof of Concept

Owner sets sellFee to max uint24. User tries selling 1000 tokens but fee calculates to 167 million tokens. Transaction reverts since fee exceeds balance, trapping all user funds.

function test_rugPullViaFee() public {
hook.ChangeFee(false, 0, true, type(uint24).max);
// Fee: (1000 * 16777215) / 100000 = 167,772,150 tokens
// User only has 1000, swap reverts
vm.expectRevert();
swapRouter.swap(key, sellParams, testSettings, "");
}

Recommended Mitigation

+ uint24 public constant MAX_FEE = 100000;
​
function ChangeFee(...) external onlyOwner {
+ require(_buyFee <= MAX_FEE && _sellFee <= MAX_FEE, "fee too high");
if(_isBuyFee) buyFee = _buyFee;
if(_isSellFee) sellFee = _sellFee;
}

Fee Denominator Causes 10x Overcharge

Description

Uniswap V4 fees use 1,000,000 as denominator where 3000 equals 0.3%. The calculation uses 100,000 as denominator, making all fees ten times higher than intended.

} else {
fee = sellFee;
// @> uint256 feeAmount = (swapAmount * sellFee) / 100000;
emit ReFiSold(sender, swapAmount, feeAmount);
}

Risk

Likelihood:

  • Every sell swap uses this calculation

  • Hardcoded denominator affects all transactions

Impact:

  • Users pay 3% instead of 0.3% on every sell

  • Extra 27,000 tokens taken per million swapped

Proof of Concept

Selling 1,000,000 tokens with sellFee=3000 calculates fee as (1000000 * 3000) / 100000 = 30,000 tokens (3%). Correct calculation with 1,000,000 denominator yields 3,000 tokens (0.3%).

function test_10xOvercharge() public {
uint256 amount = 1_000_000e18;
uint24 fee = 3000;
uint256 wrongFee = (amount * fee) / 100000; // 30,000 tokens
uint256 correctFee = (amount * fee) / 1000000; // 3,000 tokens
assertEq(wrongFee, 30_000e18);
assertEq(correctFee, 3_000e18);
}

Recommended Mitigation

- uint256 feeAmount = (swapAmount * sellFee) / 100000;
+ uint256 feeAmount = (swapAmount * sellFee) / 1000000;

Hook Returns Zero Delta Collecting No Fees

Description

Hooks collect fees by returning non-zero BeforeSwapDelta to the pool manager. The function calculates fees and emits events but always returns ZERO_DELTA, instructing the pool manager to transfer nothing.

return (
BaseHook.beforeSwap.selector,
// @> BeforeSwapDeltaLibrary.ZERO_DELTA,
fee | LPFeeLibrary.OVERRIDE_FEE_FLAG
);

Risk

Likelihood:

  • Every swap returns ZERO_DELTA regardless of calculated fee

  • Hardcoded return value

Impact:

  • Hook balance remains zero despite fee events claiming collection

  • Protocol earns no revenue from any swaps

Proof of Concept

After executing a sell swap, the ReFiSold event claims fees were collected but hook balance stays at zero. The ZERO_DELTA return told pool manager "transfer 0 tokens to hook".

function test_noFeesCollected() public {
uint256 balanceBefore = reFi.balanceOf(address(hook));
swapRouter.swap(key, sellParams, testSettings, "");
uint256 balanceAfter = reFi.balanceOf(address(hook));
assertEq(balanceBefore, 0);
assertEq(balanceAfter, 0); // still zero
}

Recommended Mitigation

uint24 fee;
+ BeforeSwapDelta delta = BeforeSwapDeltaLibrary.ZERO_DELTA;
​
if (isReFiBuy) {
fee = buyFee;
emit ReFiBought(sender, swapAmount);
} else {
fee = sellFee;
uint256 feeAmount = (swapAmount * sellFee) / 1000000;
+ poolManager.take(
+ Currency.unwrap(key.currency0) == ReFi ? key.currency0 : key.currency1,
+ address(this),
+ feeAmount
+ );
+ delta = BeforeSwapDeltaLibrary.fromAmounts(int128(uint128(feeAmount)), 0);
emit ReFiSold(sender, swapAmount, feeAmount);
}
​
return (
BaseHook.beforeSwap.selector,
- BeforeSwapDeltaLibrary.ZERO_DELTA,
+ delta,
fee | LPFeeLibrary.OVERRIDE_FEE_FLAG
);
Updates

Lead Judging Commences

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