Lines of code

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

Vulnerability details

Impact

Detailed description of the impact of this finding.

Contract: DutchTrade Function name: bid() PC address: 16558 Estimated Gas Usage: 2026 - 2971

The bid() function in the DutchTrade contract contains a critical vulnerability related to improper state transition handling, which could be exploited to disrupt the auction process.

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.

The function begins by asserting that the status is TradeStatus.OPEN.

However, the function later calls origin.settleTrade(sell), which might indirectly change the status of the contract (possibly closing the auction or changing the state).

Despite this, the function does not immediately check or enforce the status before proceeding further.

This could potentially allow an attacker to force an unexpected state transition, leading to an invalid bid being accepted or even causing the contract to revert.

function bid() external returns (uint256 amountIn) {
    require(bidder == address(0), "bid already received");
    assert(status == TradeStatus.OPEN); // Initial state check

    // {buyTok/sellTok}
    uint192 price = _price(uint48(block.timestamp)); // Calculate auction price

    // {qBuyTok}
    amountIn = _bidAmount(price); // Calculate the amount of buy tokens needed

    // Mark bidder
    bidder = msg.sender;
    bidType = BidType.TRANSFER;

    // reportViolation if auction cleared in geometric phase
    if (price > bestPrice.mul(ONE_POINT_FIVE, CEIL)) {
        broker.reportViolation();
    }

    // Transfer in buy tokens from bidder
    buy.safeTransferFrom(msg.sender, address(this), amountIn);

    // settle() in core protocol - Potential state change occurs here
    origin.settleTrade(sell);

    // confirm .settleTrade() succeeded and .settle() has been called
    assert(status == TradeStatus.CLOSED); // Re-check only after a potentially invalid state change
}

POC - Remix IDE:

Deploy contract to address(0x5B3...).

Call DutchTrade bid function from address(0xAb8...) Call bid function from another address

DutchTrade.bid();

transact to DutchTrade.bid pending ... 
[vm]from: 0xAb8...35cb2to: IAsset.(fallback) 0x5B3...eddC4value: 1000000000000000000 weidata: 0x199...8aeeflogs: 0hash: 0x0b2...a3cb8
status  0x1 Transaction mined and execution succeed
transaction hash    0x0b2addfd55daaad935afe496596c732f80625b96fbf056e2a1e5544728ba3cb8
block hash  0x37127c4a36649b7c69578b4ac82df4dfb00c250c59568544d316d74f332b3e20
block number    32
from    0xAb8483F64d9C6d1EcF9b849Ae677dD3315835cb2
to  IAsset.(fallback) 0x5B38Da6a701c568545dCfcB03FcB875f56beddC4
gas 999990000000000000 gas
transaction cost    21064 gas 
input   0x199...8aeef
decoded input   -
decoded output   - 
logs    []
raw logs    []
value   1000000000000000000 wei

Tools Used

Manual review & MythX & Remix IDE.

Recommended Mitigation Steps

To mitigate this vulnerability, it is crucial to recheck the contract’s status immediately after the origin.settleTrade(sell) call to ensure the state transition was successful and to prevent any invalid or unexpected transitions.

This recheck should ensure that the contract is in the appropriate state before proceeding to finalise the function.

function bid() external returns (uint256 amountIn) {
    require(bidder == address(0), "bid already received");
    assert(status == TradeStatus.OPEN); // Initial state check

    // Balance update made before call.
    bidder = msg.sender;
    bidType = BidType.TRANSFER;

    // {buyTok/sellTok}
    uint192 price = _price(uint48(block.timestamp)); // Calculate auction price

    // {qBuyTok}
    amountIn = _bidAmount(price); // Calculate the amount of buy tokens needed

    // reportViolation if auction cleared in geometric phase
    if (price > bestPrice.mul(ONE_POINT_FIVE, CEIL)) {
        broker.reportViolation();
    }

    // Transfer in buy tokens from bidder
    buy.safeTransferFrom(msg.sender, address(this), amountIn);

    // settle() in core protocol - Potential state change occurs here
    origin.settleTrade(sell);

    // Recheck the status immediately after settlement
    require(status == TradeStatus.CLOSED, "Unexpected state after settlement");

    // Proceed safely after confirming state
}

Additionally the balance update is moved further up before the calls to deter unauthorised calls to the bid function.

Assessed type

Invalid Validation

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

Improper State Transition Handling on the bid function in the DutchTrade contract #138