search

code-423n4 / 2024-07-reserve-validation

0 stars 0 forks source link

Potential for Undetected Unfavorable Trade Outcomes in GnosisTrade's `settle` function #248

Closed c4-bot-2 closed 1 month ago

c4-bot-2 commented 1 month ago

Lines of code

https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/plugins/trading/GnosisTrade.sol#L175

Vulnerability details

Potential for Undetected Unfavorable Trade Outcomes in GnosisTrade's settle function

Vulnerability Details

The GnosisTrade contract is designed to execute trades using the Gnosis EasyAuction mechanism. The settle function finalizes the trade and includes a mechanism to check if the trade outcome meets the expected worst-case price.

The issue lies in how the clearing price is calculated and compared to the worst-case price. The function uses adjusted values for sold and bought amounts, which could potentially mask unfavorable trade outcomes in certain scenarios.

Code snippet

function settle()
    external
    stateTransition(TradeStatus.OPEN, TradeStatus.CLOSED)
    returns (uint256 soldAmt, uint256 boughtAmt)
{
    // ...

    // Check clearing prices
    if (sellBal < initBal) {
        soldAmt = initBal - sellBal;

        // Gnosis rounds defensively in the buy token; we should not consider it a violation
        uint256 adjustedSoldAmt = Math.max(soldAmt, 1);
        uint256 adjustedBuyAmt = boughtAmt + 1;

        // D27{buyTok/sellTok}
        uint192 clearingPrice = shiftl_toFix(adjustedBuyAmt, 9).divu(adjustedSoldAmt, FLOOR);
        if (clearingPrice.lt(worstCasePrice)) broker.reportViolation();
    }
}

Impact

  1. Possible Failure to Detect Unfavorable Trades: In certain edge cases, particularly with small trade amounts, the adjustments made to soldAmt and boughtAmt could prevent the detection of trades that actually performed worse than the worst-case price.
  2. False Sense of Security: Users and the system may believe that all unfavorable trades are being caught, when in reality, some might slip through due to these adjustments.

Scenario

  1. Let's say the worst-case price is set to 1 buy token per 1 sell token.
  2. The auction concludes with a slightly unfavorable outcome: 99 sell tokens for 98 buy tokens.
  3. In the settle function:
    • soldAmt = 99
    • boughtAmt = 98
    • adjustedSoldAmt = max(99, 1) = 99
    • adjustedBuyAmt = 98 + 1 = 99
  4. The calculated clearingPrice becomes 99/99 = 1, which is not less than the worst-case price.
  5. No violation is reported, even though the actual trade was unfavorable.

Mitigation

  1. Consider removing or refining the adjustments made to soldAmt and boughtAmt.
  2. Implement a more precise comparison that accounts for the actual amounts without adjustments.
  3. Add additional checks or a tolerance factor to catch edge cases where the current mechanism might fail.

Assessed type

Context