SIZE contest - __141345__'s results

Lines of code

https://github.com/code-423n4/2022-11-size/blob/79aa9c01987e57a760521acecfe81b28eab3b313/src/SizeSealed.sol#L163 https://github.com/code-423n4/2022-11-size/blob/79aa9c01987e57a760521acecfe81b28eab3b313/src/SizeSealed.sol#L321 https://github.com/code-423n4/2022-11-size/blob/79aa9c01987e57a760521acecfe81b28eab3b313/src/SizeSealed.sol#L327 https://github.com/code-423n4/2022-11-size/blob/79aa9c01987e57a760521acecfe81b28eab3b313/src/SizeSealed.sol#L351 https://github.com/code-423n4/2022-11-size/blob/79aa9c01987e57a760521acecfe81b28eab3b313/src/SizeSealed.sol#L381 https://github.com/code-423n4/2022-11-size/blob/79aa9c01987e57a760521acecfe81b28eab3b313/src/SizeSealed.sol#L384 https://github.com/code-423n4/2022-11-size/blob/79aa9c01987e57a760521acecfe81b28eab3b313/src/SizeSealed.sol#L409 https://github.com/code-423n4/2022-11-size/blob/79aa9c01987e57a760521acecfe81b28eab3b313/src/SizeSealed.sol#L439

Vulnerability details

Impact

Some ERC20 may be tricky for the balance. Such as:

• fee on transfer (USDT has this mode)
• rebasing (aToken from AAVE)
• variable balance (stETH)

For these tokens, the balance can change even without transfer()/transferFrom(). Users fund could be locked:

• when the balance decrease, the protocol could have DoS due to lack of fund.
• if the balance increases, the excess amount above the booking value can not be withdrawn.

As long as one of the base token and quote token have this issue, the whole contract could be affected.

Proof of Concept

Before and after transfer balance is checked in createAuction(), however none of the following functions take into account the potential changes in token balance:

• bid()
• finalize()
• refund()
• withdraw()
• cancelAuction()
• cancelBid()

Tools Used

Manual analysis.

Recommended Mitigation Steps

• checking the before and after balance of token transfer
• recording the relative shares of each user instead of specific amount
• disallow such kind of tokens

Lines of code

https://github.com/code-423n4/2022-11-size/blob/79aa9c01987e57a760521acecfe81b28eab3b313/src/SizeSealed.sol#L157-L159 https://github.com/code-423n4/2022-11-size/blob/79aa9c01987e57a760521acecfe81b28eab3b313/src/SizeSealed.sol#L415-L440

Vulnerability details

Impact

Cancelled bid is not deleted from a.bids, so the a.bids array can not be shortened. Eventually it could exceed the maximum 1000 bids number and no new bid can be placed.

Proof of Concept

The bidder's first bid A malicious bidder can firstly place a bid at reserveQuotePerBase. Then repeatedly create and cancel bid, until the bids[] array reaches the maximum length. So no new bids can be added.

// src/SizeSealed.sol
    function bid() {
        // ...
        if (bidIndex >= 1000) {
            revert InvalidState();
        }
        // ...
    }

As a result, the whole auction will not function as expected. Either sell the base token to the malicious user at the reserveQuotePerBase, or just cancel the auction.

Tools Used

Manual analysis.

Recommended Mitigation Steps

In cancelBid(), delete the bid from the a.bids array if the bid is cancelled.