Skip to content

Latest commit

 

History

History
151 lines (103 loc) · 4.03 KB

021.md

File metadata and controls

151 lines (103 loc) · 4.03 KB

Flaky Merlot Parrot

Medium

Unvalidated Fee Basis Points in performUpkeep Will Cause Excessive Fees for Users

Summary

The absence of validation for fee basis points (feeBips) in performUpkeep and applyFee will cause excessive fees for users as malicious actors or misconfigurations exploit the lack of a maximum limit.

Root Cause

1- Unbounded Fee Application:

The applyFee function calculates fees using feeBips without validating whether it is within acceptable bounds: https://github.com/sherlock-audit/2024-11-oku/blob/main/oku-custom-order-types/contracts/automatedTrigger/Bracket.sol#L624-L625

  • This allows excessively high or invalid feeBips values to be used.

2- Missing Validation in performUpkeep:

The performUpkeep function directly passes order.feeBips to applyFee without validating its value: https://github.com/sherlock-audit/2024-11-oku/blob/main/oku-custom-order-types/contracts/automatedTrigger/Bracket.sol#L125-L128

3- No Constraints During Order Creation:

  • There are no safeguards during order creation or update to ensure feeBips values are reasonable.

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

Attack Path 1- A malicious actor creates or modifies an order with feeBips = 10,000 (100% fee). 2- The order is executed via performUpkeep, and applyFee is called:

//performupKeep()
(uint256 feeAmount, uint256 adjustedAmount) = applyFee(swapAmountOut, 10_000);

3- The applyFee function calculates:

//applyFee() in Brackets Contract
adjustedAmount = (swapAmountOut * (10000 - 10_000)) / 10_000; // adjustedAmount = 0
feeAmount = swapAmountOut - adjustedAmount;                   // feeAmount = swapAmountOut

The entire swapAmountOut is deducted as a fee, leaving the user with no tokens.

Step-by-Step Issue Breakdown

Function: performUpkeep Role: Executes trades and applies fees for completed orders.

Vulnerable Code:

(uint256 feeAmount, uint256 adjustedAmount) = applyFee(
    swapAmountOut,
    order.feeBips
);

Impact: If feeBips is unreasonably high (e.g., 10,000 for 100%), the entire swapAmountOut could be deducted as a fee.

Function: applyFee Role: Calculates and applies the fee based on feeBips.

Vulnerable Code:

adjustedAmount = (amount * (10000 - feeBips)) / 10000;
feeAmount = amount - adjustedAmount;

Impact: The calculation assumes feeBips is valid but does not enforce any constraints. Invalid or excessively high feeBips values directly impact the calculated fee and adjusted amount.

Impact

Excessive Fees: Users are charged unreasonable fees, potentially losing all their swapped tokens.

Loss of Trust: Users lose confidence in the platform’s reliability and fairness.

Exploitation Potential: Malicious actors can misuse high feeBips configurations to harm users or the system.

PoC

Scenario:

1- Create an order with: swapAmountOut = 1,000 tokens. feeBips = 10,000 (100%). 2- During execution, the applyFee function calculates:

adjustedAmount = (1_000 * (10_000 - 10_000)) / 10_000; // adjustedAmount = 0
feeAmount = 1_000 - adjustedAmount;                    // feeAmount = 1_000

3- The user receives no tokens (adjustedAmount = 0), and the entire swapAmountOut is deducted as a fee.

Mitigation

Proposed Fix

Fix in applyFee():

function applyFee(
    uint256 amount,
    uint16 feeBips
) internal pure returns (uint256 feeAmount, uint256 adjustedAmount) {
    require(feeBips <= MAX_FEE_BIPS, "Excessive fee bips");
    if (feeBips != 0) {
        adjustedAmount = (amount * (10000 - feeBips)) / 10000;
        feeAmount = amount - adjustedAmount;
    } else {
        return (0, amount);
    }
}

Validation in performUpkeep:

(uint256 feeAmount, uint256 adjustedAmount) = applyFee(
    swapAmountOut,
    order.feeBips
);
require(feeAmount <= swapAmountOut, "Fee exceeds amount");

Validation During Order Creation:

require(feeBips <= MAX_FEE_BIPS, "Fee bips too high");