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

0 stars 0 forks source link

WETH compatibility in Arbitrum #80

Closed c4-bot-3 closed 1 month ago

c4-bot-3 commented 2 months ago

Lines of code

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

Vulnerability details

Impact

Users can't bid with WETH tokens in arbitrum

Proof of Concept

During a Auction when users need to bid with their buying tokens, it will revert on chains like Arbitrum due to a different implementation of WETH

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

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

        // {qBuyTok}
        amountIn = _bidAmount(price);

        // 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
        origin.settleTrade(sell);

        // confirm .settleTrade() succeeded and .settle() has been called
        assert(status == TradeStatus.CLOSED);
    }

The token transfer is done using the safeTransferFrom method. This works fine on most chains (Ethereum, Optimism, Polygon, BSC) which use the standard WETH9 contract that handles the case src == msg.sender:

WETH9.sol

if (src != msg.sender && allowance[src][msg.sender] != uint(- 1)) {
            require(allowance[src][msg.sender] >= wad);
            allowance[src][msg.sender] -= wad;
        }

The problem is that the WETH implementation on Arbitrum uses a different contract, and does not have this src == msg.sender handling.

Tools Used

Manual Review

Recommended Mitigation Steps

Change the following logic

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

Assessed type

ERC20