Skip to content

Latest commit

 

History

History
92 lines (52 loc) · 2.53 KB

File metadata and controls

92 lines (52 loc) · 2.53 KB

Mammoth Mocha Koala

Medium

Inverted Taker Fee Logic in from()

Summary

https://github.com/sherlock-audit/2025-01-perennial-v2-4-update/blob/main/perennial-v2/packages/core/contracts/types/Guarantee.sol#L63C5-L78C6

The from() function is intended to create a Guarantee from an Order, including determining whether a taker fee should be charged. However, the logic for setting takerFee is inverted, leading to incorrect fee exemptions.

Root Cause

https://github.com/sherlock-audit/2025-01-perennial-v2-4-update/blob/main/perennial-v2/packages/core/contracts/types/Guarantee.sol#L63C5-L78C6

function from(
    Order memory order,
    Fixed6 priceOverride,
    UFixed6 solverReferralFee,
    bool chargeTradeFee
) internal pure returns (Guarantee memory newGuarantee) {
    newGuarantee.orders = order.orders;


    (newGuarantee.longPos, newGuarantee.longNeg, newGuarantee.shortPos, newGuarantee.shortNeg) =
        (order.longPos, order.longNeg, order.shortPos, order.shortNeg);
    newGuarantee.takerFee = chargeTradeFee ? UFixed6Lib.ZERO : order.takerTotal();


    newGuarantee.notional = taker(newGuarantee).mul(priceOverride);
    newGuarantee.orderReferral = order.takerReferral;
    newGuarantee.solverReferral = order.takerReferral.mul(solverReferralFee);
}

newGuarantee.takerFee = chargeTradeFee ? UFixed6Lib.ZERO : order.takerTotal(); // ❌ chargeTradeFee: A boolean flag indicating whether the order should incur a taker fee.

Intended Behavior:

If chargeTradeFee is true: The order should pay the fee (takerFee = order.takerTotal()).

If chargeTradeFee is false: The order should be exempt (takerFee = 0).

Actual Behavior:

The code does the opposite: It sets takerFee to 0 when chargeTradeFee is true, and charges the fee when chargeTradeFee is false.

Example Order Parameters:

chargeTradeFee = true (fee should apply).

order.takerTotal() = 10 (fee amount).

Result:

takerFee = 0 (fee is waived despite chargeTradeFee being true).

Internal Pre-conditions

No response

External Pre-conditions

No response

Attack Path

No response

Impact

The protocol loses expected revenue.

Users bypass fees they should pay, violating system invariants.

PoC

No response

Mitigation

Flip the ternary condition to align with the intended logic:

newGuarantee.takerFee = chargeTradeFee ? order.takerTotal() : UFixed6Lib.ZERO; // Corrected Behavior chargeTradeFee = true → Fee charged (takerFee = order.takerTotal()).

chargeTradeFee = false → Fee exempt (takerFee = 0).