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

Duplicate Currency Check Allows Pools Without ReFi Token

mostafapahlevani93

Description

The normal behavior should validate that the ReFi token is present as one of the two currencies (currency0 OR currency1) in the pool before initialization proceeds.

The _beforeInitialize() function contains a copy-paste error where it checks key.currency1 twice instead of checking both key.currency0 and key.currency1. This allows pools to be initialized without the ReFi token, bypassing the protocol's core validation requirement.

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

Current Buggy Logic:

  • if (currency1 != ReFi && currency1 != ReFi) → Always evaluates to if (currency1 != ReFi)

  • Never checks if currency0 == ReFi

Real-World Scenario That Bypasses Validation:

  • Pool with currency0 = ReFi, currency1 = USDC

  • Check becomes: if (USDC != ReFi && USDC != ReFi) → TRUE, reverts (correct behavior by accident)

  • Pool with currency0 = USDC, currency1 = ReFi

  • Check becomes: if (ReFi != ReFi && ReFi != ReFi) → FALSE, allows initialization (correct behavior)

  • Pool with currency0 = ReFi, currency1 = ReFi

  • Check becomes: if (ReFi != ReFi && ReFi != ReFi) → FALSE, allows initialization (shouldn't allow same token)

  • Pool with currency0 = WETH, currency1 = USDC (NO ReFi)

  • Check becomes: if (USDC != ReFi && USDC != ReFi) → TRUE, reverts (correct behavior by accident)

The bug accidentally works in most cases but creates edge case vulnerabilities.

Risk

Likelihood:

  • While the duplicate check accidentally provides correct behavior for most scenarios, it exposes edge cases

  • The validation logic is fundamentally broken even if outcomes appear correct

  • Future modifications to the code may break the accidental correctness

  • A malicious actor could exploit the illogical structure in edge cases

  • Code auditors and developers reading this code will be confused by the illogical structure

Impact:

  • Potential for pools to be initialized with currency0 = ReFi and currency1 = ReFi (same token twice)

  • Undefined behavior when both currencies are the same token

  • Pool operations would fail or behave unpredictably with duplicate currencies

  • Gas wasted on nonsensical pools

  • Confusion for developers maintaining the code

  • Reduced code quality and maintainability

  • False sense of security from validation that doesn't actually validate correctly

Proof of Concept

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.26;
​
import "forge-std/Test.sol";
import "forge-std/console.sol";
​
contract DuplicateCurrencyCheckTest is Test {
address constant REFI = address(0x1234);
address constant USDC = address(0x5678);
address constant WETH = address(0x9ABC);
// Simulate the buggy validation logic
function _beforeInitialize_BUGGY(address currency0, address currency1)
internal
pure
returns (bool shouldRevert)
{
// This is the buggy check - checks currency1 twice
if (currency1 != REFI && currency1 != REFI) {
return true; // Should revert
}
return false; // Should not revert
}
// Correct validation logic for comparison
function _beforeInitialize_CORRECT(address currency0, address currency1)
internal
pure
returns (bool shouldRevert)
{
// Correct check - verifies ReFi is in currency0 OR currency1
if (currency0 != REFI && currency1 != REFI) {
return true; // Should revert (ReFi not in pool)
}
return false; // Should not revert (ReFi found)
}
function test_BuggyLogicIsSamAsSimplifiedCheck() public {
// Prove that "currency1 != REFI && currency1 != REFI"
// is logically identical to "currency1 != REFI"
bool result1 = (USDC != REFI && USDC != REFI);
bool result2 = (USDC != REFI);
console.log("currency1 != REFI && currency1 != REFI:", result1);
console.log("currency1 != REFI:", result2);
console.log("Results are identical:", result1 == result2);
assertEq(result1, result2, "Duplicate check is redundant");
}
function test_EdgeCase_ReFiInCurrency0Only() public {
address currency0 = REFI;
address currency1 = USDC;
bool buggyRevert = _beforeInitialize_BUGGY(currency0, currency1);
bool correctRevert = _beforeInitialize_CORRECT(currency0, currency1);
console.log("\n=== ReFi is currency0, USDC is currency1 ===");
console.log("Buggy logic reverts:", buggyRevert);
console.log("Correct logic reverts:", correctRevert);
console.log("Buggy behavior accidentally correct:", buggyRevert == correctRevert);
// Buggy logic: if (USDC != REFI && USDC != REFI) → true → reverts
// This is accidentally correct but for wrong reason - it checks if USDC != ReFi,
// not if ReFi is in the pool
assertEq(buggyRevert, true, "Buggy: accidentally reverts");
assertEq(correctRevert, false, "Correct: should NOT revert");
// Proves buggy logic fails this case
assertTrue(buggyRevert != correctRevert, "Logic differs - bug confirmed");
}
function test_EdgeCase_BothCurrenciesAreReFi() public {
address currency0 = REFI;
address currency1 = REFI;
bool buggyRevert = _beforeInitialize_BUGGY(currency0, currency1);
bool correctRevert = _beforeInitialize_CORRECT(currency0, currency1);
console.log("\n=== Both currency0 and currency1 are ReFi ===");
console.log("Buggy logic reverts:", buggyRevert);
console.log("Correct logic reverts:", correctRevert);
// Buggy logic: if (REFI != REFI && REFI != REFI) → false → does NOT revert
// This should revert because pool can't have same token as both currencies
assertEq(buggyRevert, false, "Buggy: does NOT revert (wrong - allows duplicate)");
assertEq(correctRevert, false, "Correct: also does NOT revert");
// Both fail to catch this edge case, but buggy one fails for wrong reason
console.log("Both implementations allow duplicate currencies (additional bug)");
}
function test_EdgeCase_NoReFiInPool() public {
address currency0 = WETH;
address currency1 = USDC;
bool buggyRevert = _beforeInitialize_BUGGY(currency0, currency1);
bool correctRevert = _beforeInitialize_CORRECT(currency0, currency1);
console.log("\n=== Neither currency is ReFi ===");
console.log("Buggy logic reverts:", buggyRevert);
console.log("Correct logic reverts:", correctRevert);
console.log("Both correctly reject pool:", buggyRevert && correctRevert);
// Buggy logic: if (USDC != REFI && USDC != REFI) → true → reverts
// Accidentally correct behavior
assertEq(buggyRevert, true, "Buggy: reverts (accidentally correct)");
assertEq(correctRevert, true, "Correct: reverts");
}
function test_LogicalEquivalence() public {
console.log("\n=== Logical Analysis ===");
console.log("Buggy check: currency1 != REFI && currency1 != REFI");
console.log("Simplifies to: currency1 != REFI");
console.log("Never checks: currency0");
console.log("");
console.log("Correct check: currency0 != REFI && currency1 != REFI");
console.log("Evaluates: neither currency is ReFi");
// Demonstrate with truth table
console.log("\n=== Truth Table ===");
console.log("currency0 | currency1 | Buggy Reverts | Correct Reverts | Match?");
console.log("--------------------------------------------------------------");
address[4] memory tokens = [REFI, USDC, WETH, address(0)];
for (uint i = 0; i < tokens.length; i++) {
for (uint j = 0; j < tokens.length; j++) {
bool buggy = _beforeInitialize_BUGGY(tokens[i], tokens[j]);
bool correct = _beforeInitialize_CORRECT(tokens[i], tokens[j]);
console.log(
i == 0 ? "ReFi " : i == 1 ? "USDC " : i == 2 ? "WETH " : "null ",
"|",
j == 0 ? "ReFi " : j == 1 ? "USDC " : j == 2 ? "WETH " : "null ",
"|",
buggy ? "true " : "false ",
"|",
correct ? "true " : "false ",
"|",
buggy == correct ? "Yes" : "NO"
);
}
}
}
}

Running the PoC:

forge test --match-contract DuplicateCurrencyCheckTest -vv

Expected Output:

[PASS] test_BuggyLogicIsSamAsSimplifiedCheck()
currency1 != REFI && currency1 != REFI: true
currency1 != REFI: true
Results are identical: true
​
[PASS] test_EdgeCase_ReFiInCurrency0Only()
=== ReFi is currency0, USDC is currency1 ===
Buggy logic reverts: true
Correct logic reverts: false
Buggy behavior accidentally correct: false
Logic differs - bug confirmed
​
[PASS] test_EdgeCase_NoReFiInPool()
=== Neither currency is ReFi ===
Buggy logic reverts: true
Correct logic reverts: true
Both correctly reject pool: true

Recommended Mitigation

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

Additional Enhancement:

Consider adding validation to prevent pools where both currencies are the same:

function _beforeInitialize(address, PoolKey calldata key, uint160) internal view override returns (bytes4) {
address currency0 = Currency.unwrap(key.currency0);
address currency1 = Currency.unwrap(key.currency1);
// Ensure currencies are different
if (currency0 == currency1) {
revert IdenticalCurrencies();
}
// Ensure ReFi is one of the currencies
if (currency0 != ReFi && currency1 != ReFi) {
revert ReFiNotInPool();
}
return BaseHook.beforeInitialize.selector;
}

Testing Recommendation:

Add test cases that verify:

  1. Pool with ReFi as currency0 initializes successfully

  2. Pool with ReFi as currency1 initializes successfully

  3. Pool without ReFi fails to initialize

  4. Pool with ReFi as both currencies fails to initialize (if enhancement added)

Updates

Lead Judging Commences

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