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

Inverted Buy/Sell Fee Logic Causes Opposite Fee Application

mostafapahlevani93

Description

The normal behavior should apply 0% fee when users buy ReFi tokens and 0.3% fee when users sell ReFi tokens to incentivize accumulation and discourage dumping.

The _isReFiBuy() function contains inverted logic that incorrectly determines swap direction. When ReFi is currency0 and a user swaps currency0 for currency1 (selling ReFi), the function incorrectly returns true indicating a buy. This causes the protocol to apply buy fees (0%) to sell transactions and sell fees (0.3%) to buy transactions - the complete opposite of intended behavior.

function _isReFiBuy(PoolKey calldata key, bool zeroForOne) internal view returns (bool) {
bool IsReFiCurrency0 = Currency.unwrap(key.currency0) == ReFi;
if (IsReFiCurrency0) {
return zeroForOne; // ❌ WRONG: zeroForOne=true means selling currency0 (ReFi), not buying
} else {
return !zeroForOne; // ❌ WRONG: zeroForOne=false means selling currency1 (ReFi), not buying
}
}

Uniswap V4 Swap Direction Logic:

  • zeroForOne = true → Swapping currency0 FOR currency1 (SELLING currency0)

  • zeroForOne = false → Swapping currency1 FOR currency0 (SELLING currency1)

Current Buggy Behavior:

  • If ReFi is currency0 and zeroForOne=true → Returns true (claims "buy") but user is SELLING ReFi

  • If ReFi is currency1 and zeroForOne=false → Returns true (claims "buy") but user is SELLING ReFi

Risk

Likelihood:

  • Every single swap through pools using this hook will apply the wrong fee

  • The bug is deterministic and affects 100% of transactions

  • All existing test cases pass because they don't verify actual fee amounts charged, only that fees are configured

  • The protocol will immediately apply opposite fees upon first production swap

Impact:

  • Protocol completely fails its core economic model - incentivizes selling instead of buying

  • Users buying ReFi tokens are charged 0.3% fee instead of 0% fee, discouraging accumulation

  • Users selling ReFi tokens pay 0% fee instead of 0.3% fee, encouraging dumping

  • Liquidity providers lose 0.3% fee revenue on all sell transactions

  • ReFi token price likely to crash due to incentivized selling and penalized buying

  • Protocol revenue model completely broken - no fees collected from sellers

  • Complete economic collapse of the tokenomics design

  • Reputational damage and loss of user trust

Proof of Concept

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.26;
​
import "forge-std/Test.sol";
import "forge-std/console.sol";
​
contract BuySellLogicTest is Test {
address constant REFI = address(0x1234);
// Simulate the buggy _isReFiBuy function
function _isReFiBuy_BUGGY(address currency0, address currency1, bool zeroForOne)
internal
pure
returns (bool)
{
bool IsReFiCurrency0 = currency0 == REFI;
if (IsReFiCurrency0) {
return zeroForOne;
} else {
return !zeroForOne;
}
}
function test_BugProof_ReFiIsCurrency0() public {
address currency0 = REFI;
address currency1 = address(0xABCD); // Some other token (e.g., USDC)
// Scenario 1: User wants to SELL ReFi for USDC
// zeroForOne = true means swapping currency0 (ReFi) for currency1 (USDC)
// This is SELLING ReFi, should apply SELL fee (3000 = 0.3%)
bool zeroForOne = true;
bool isBuy = _isReFiBuy_BUGGY(currency0, currency1, zeroForOne);
console.log("ReFi is currency0, zeroForOne=true (SELLING ReFi):");
console.log(" Buggy function returns isBuy:", isBuy);
console.log(" Expected: false (should apply SELL fee)");
console.log(" Actual: true (applies BUY fee - WRONG!)");
assertEq(isBuy, true); // Bug confirmed: returns true when should be false
// Scenario 2: User wants to BUY ReFi with USDC
// zeroForOne = false means swapping currency1 (USDC) for currency0 (ReFi)
// This is BUYING ReFi, should apply BUY fee (0 = 0%)
zeroForOne = false;
isBuy = _isReFiBuy_BUGGY(currency0, currency1, zeroForOne);
console.log("\nReFi is currency0, zeroForOne=false (BUYING ReFi):");
console.log(" Buggy function returns isBuy:", isBuy);
console.log(" Expected: true (should apply BUY fee)");
console.log(" Actual: false (applies SELL fee - WRONG!)");
assertEq(isBuy, false); // Bug confirmed: returns false when should be true
}
function test_BugProof_ReFiIsCurrency1() public {
address currency0 = address(0xABCD); // Some other token (e.g., USDC)
address currency1 = REFI;
// Scenario 1: User wants to BUY ReFi with USDC
// zeroForOne = true means swapping currency0 (USDC) for currency1 (ReFi)
// This is BUYING ReFi, should apply BUY fee (0 = 0%)
bool zeroForOne = true;
bool isBuy = _isReFiBuy_BUGGY(currency0, currency1, zeroForOne);
console.log("ReFi is currency1, zeroForOne=true (BUYING ReFi):");
console.log(" Buggy function returns isBuy:", isBuy);
console.log(" Expected: true (should apply BUY fee)");
console.log(" Actual: false (applies SELL fee - WRONG!)");
assertEq(isBuy, false); // Bug confirmed: returns false when should be true
// Scenario 2: User wants to SELL ReFi for USDC
// zeroForOne = false means swapping currency1 (ReFi) for currency0 (USDC)
// This is SELLING ReFi, should apply SELL fee (3000 = 0.3%)
zeroForOne = false;
isBuy = _isReFiBuy_BUGGY(currency0, currency1, zeroForOne);
console.log("\nReFi is currency1, zeroForOne=false (SELLING ReFi):");
console.log(" Buggy function returns isBuy:", isBuy);
console.log(" Expected: false (should apply SELL fee)");
console.log(" Actual: true (applies BUY fee - WRONG!)");
assertEq(isBuy, true); // Bug confirmed: returns true when should be false
}
function test_EconomicImpact() public {
// Demonstrate the economic impact
uint24 buyFee = 0;
uint24 sellFee = 3000;
address currency0 = REFI;
address currency1 = address(0xABCD);
// User selling 1000 ReFi tokens
bool zeroForOne = true; // Selling currency0 (ReFi)
bool isBuy = _isReFiBuy_BUGGY(currency0, currency1, zeroForOne);
uint24 appliedFee = isBuy ? buyFee : sellFee;
console.log("\n=== Economic Impact ===");
console.log("User sells 1000 ReFi (should pay 0.3% fee):");
console.log(" Applied fee:", appliedFee, "basis points");
console.log(" Expected fee: 3000 basis points (0.3%)");
console.log(" Fee charged: 0% instead of 0.3%");
console.log(" LP revenue lost: 0.3% of trade");
assertEq(appliedFee, 0); // Proves sellers pay 0% instead of 0.3%
// User buying 1000 ReFi tokens
zeroForOne = false; // Buying currency0 (ReFi)
isBuy = _isReFiBuy_BUGGY(currency0, currency1, zeroForOne);
appliedFee = isBuy ? buyFee : sellFee;
console.log("\nUser buys 1000 ReFi (should pay 0% fee):");
console.log(" Applied fee:", appliedFee, "basis points");
console.log(" Expected fee: 0 basis points (0%)");
console.log(" Fee charged: 0.3% instead of 0%");
console.log(" Buyer penalized with unexpected 0.3% fee");
assertEq(appliedFee, 3000); // Proves buyers pay 0.3% instead of 0%
}
}

Running the PoC:

forge test --match-contract BuySellLogicTest -vv

Expected Output:

[PASS] test_BugProof_ReFiIsCurrency0()
ReFi is currency0, zeroForOne=true (SELLING ReFi):
Buggy function returns isBuy: true
Expected: false (should apply SELL fee)
Actual: true (applies BUY fee - WRONG!)
ReFi is currency0, zeroForOne=false (BUYING ReFi):
Buggy function returns isBuy: false
Expected: true (should apply BUY fee)
Actual: false (applies SELL fee - WRONG!)
​
[PASS] test_BugProof_ReFiIsCurrency1()
ReFi is currency1, zeroForOne=true (BUYING ReFi):
Buggy function returns isBuy: false
Expected: true (should apply BUY fee)
Actual: false (applies SELL fee - WRONG!)
ReFi is currency1, zeroForOne=false (SELLING ReFi):
Buggy function returns isBuy: true
Expected: false (should apply SELL fee)
Actual: true (applies BUY fee - WRONG!)
​
[PASS] test_EconomicImpact()
=== Economic Impact ===
User sells 1000 ReFi (should pay 0.3% fee):
Applied fee: 0 basis points
Expected fee: 3000 basis points (0.3%)
Fee charged: 0% instead of 0.3%
LP revenue lost: 0.3% of trade
User buys 1000 ReFi (should pay 0% fee):
Applied fee: 3000 basis points
Expected fee: 0 basis points (0%)
Fee charged: 0.3% instead of 0%
Buyer penalized with unexpected 0.3% fee

Recommended Mitigation

function _isReFiBuy(PoolKey calldata key, bool zeroForOne) internal view returns (bool) {
bool IsReFiCurrency0 = Currency.unwrap(key.currency0) == ReFi;
if (IsReFiCurrency0) {
- return zeroForOne;
+ return !zeroForOne; // If ReFi is currency0, buying means zeroForOne=false
} else {
- return !zeroForOne;
+ return zeroForOne; // If ReFi is currency1, buying means zeroForOne=true
}
}

Explanation:

  • When ReFi is currency0:

    • zeroForOne=true → Selling currency0 (ReFi) → isBuy should be FALSE

    • zeroForOne=false → Buying currency0 (ReFi) → isBuy should be TRUE

  • When ReFi is currency1:

    • zeroForOne=true → Buying currency1 (ReFi) → isBuy should be TRUE

    • zeroForOne=false → Selling currency1 (ReFi) → isBuy should be FALSE

Additional Recommendation:
Add comprehensive integration tests that verify the actual fees charged (not just fee configuration) for both buy and sell scenarios with ReFi as both currency0 and currency1.

Updates

Lead Judging Commences

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