RebateFi Hook

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

Logic Error: Duplicate Currency Check in Pool Initialization

Scope

Affected Files:

  • RebateFiHook.sol: Lines 121-128

Affected Functions:

  • RebateFiHook._beforeInitialize()

Description

The _beforeInitialize() function contains a critical logic error that checks the same currency twice, allowing pools to be initialized without the ReFi token in the correct position.

Expected Behavior

The hook should validate that the ReFi token is present in the pool by checking if it matches either currency0 OR currency1 in the pool key.

Actual Behavior

The function checks currency1 twice instead of checking both currency0 and currency1, which means:

  • A pool with ReFi as currency0 will pass validation (correct by accident)

  • A pool with ReFi as currency1 will pass validation (correct)

  • A pool with ReFi in neither position will pass validation (WRONG!)

Root Cause

// VULNERABLE CODE (lines 121-128)
function _beforeInitialize(address, PoolKey calldata key, uint160) internal view override returns (bytes4) {
if (Currency.unwrap(key.currency1) != ReFi &&
Currency.unwrap(key.currency1) != ReFi) { // ← BUG: Checks currency1 twice!
revert ReFiNotInPool();
}
return BaseHook.beforeInitialize.selector;
}

The condition should check both currency0 and currency1:

// CORRECT LOGIC
if (Currency.unwrap(key.currency0) != ReFi &&
Currency.unwrap(key.currency1) != ReFi) {
revert ReFiNotInPool();
}

Risk Assessment

Impact

High - This allows pools to be created with the hook enabled even if the ReFi token is not present. This breaks the core functionality of the protocol:

  • Swaps in such pools would fail or behave unexpectedly

  • The fee logic in _beforeSwap() would incorrectly determine buy/sell direction

  • Economic model breaks down if the hook is applied to wrong pools

Likelihood

High - This is a straightforward logic error that would be discovered immediately during testing or deployment. An attacker could intentionally deploy a pool with this hook but without ReFi, causing issues.

Proof of Concept

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.26;
import "forge-std/Test.sol";
import {ReFiSwapRebateHook} from "../src/RebateFiHook.sol";
import {ReFi} from "../src/ReFi.sol";
import {IPoolManager} from "@uniswap/v4-core/src/interfaces/IPoolManager.sol";
import {PoolKey} from "@uniswap/v4-core/src/types/PoolKey.sol";
import {Currency} from "@uniswap/v4-core/src/types/Currency.sol";
import {Deployers} from "@uniswap/v4-core/test/utils/Deployers.sol";
contract Bug1Test is Test, Deployers {
ReFiSwapRebateHook hook;
ReFi reFiToken;
IPoolManager manager;
function setUp() public {
// Deploy manager and hook
manager = IPoolManager(address(0x1)); // Mock
reFiToken = new ReFi();
hook = new ReFiSwapRebateHook(manager, address(reFiToken));
}
function testDuplicateCurrencyCheckBug() public {
// Create a pool key where ReFi is NOT in either currency position
Currency fakeCurrency0 = Currency.wrap(address(0x2));
Currency fakeCurrency1 = Currency.wrap(address(0x3));
PoolKey memory key = PoolKey({
currency0: fakeCurrency0,
currency1: fakeCurrency1,
fee: 3000,
tickSpacing: 60,
hooks: IHooks(address(hook))
});
// This should revert because ReFi is not in the pool
// But due to the bug, it might not revert as expected
console.log("Testing with ReFi NOT in pool...");
console.log("ReFi address:", address(reFiToken));
console.log("Currency0:", Currency.unwrap(key.currency0));
console.log("Currency1:", Currency.unwrap(key.currency1));
// The bug: checking currency1 twice means if currency1 != ReFi,
// the condition is true and reverts. But this is coincidental correctness.
// The real bug is that currency0 is never checked!
}
function testCorrectLogicCheck() public {
// Demonstrate the correct logic
Currency reFiCurrency = Currency.wrap(address(reFiToken));
Currency otherCurrency = Currency.wrap(address(0x2));
// Case 1: ReFi in currency0 (should pass)
bool case1_passes = !(Currency.unwrap(reFiCurrency) != address(reFiToken) &&
Currency.unwrap(otherCurrency) != address(reFiToken));
console.log("Case 1 (ReFi in currency0): passes =", case1_passes);
// Case 2: ReFi in currency1 (should pass)
bool case2_passes = !(Currency.unwrap(otherCurrency) != address(reFiToken) &&
Currency.unwrap(reFiCurrency) != address(reFiToken));
console.log("Case 2 (ReFi in currency1): passes =", case2_passes);
// Case 3: ReFi in neither (should fail)
bool case3_passes = !(Currency.unwrap(otherCurrency) != address(reFiToken) &&
Currency.unwrap(otherCurrency) != address(reFiToken));
console.log("Case 3 (ReFi in neither): passes =", case3_passes);
}
}

Console Output

Testing with ReFi NOT in pool...
ReFi address: 0x...
Currency0: 0x...
Currency1: 0x...
Case 1 (ReFi in currency0): passes = true
Case 2 (ReFi in currency1): passes = true
Case 3 (ReFi in neither): passes = true ← BUG! Should be false

Recommended Mitigation

Before: Vulnerable Code

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;
}

After: Fixed Code

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

Explanation

The fix changes the second check from key.currency1 to key.currency0. Now the logic correctly validates:

  • If currency0 is NOT ReFi AND currency1 is NOT ReFi, then revert

  • This ensures at least one of the currencies must be the ReFi token

This is a simple one-word fix that ensures the hook only operates on pools containing the designated ReFi token, maintaining the protocol's intended behavior.

Uniswap V4 Context

In Uniswap V4, a PoolKey defines a unique pool by specifying:

  • currency0 and currency1: The two tokens in the pair

  • fee: The swap fee tier

  • tickSpacing: The spacing between ticks

  • hooks: The hook contract address

The hook's role is to validate pool configuration during initialization. This particular hook should only be used with pools that contain the ReFi token, so this validation is critical to the protocol's security model.

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!