SIZE contest - wagmi's results

Lines of code

https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/DBR.sol#L288

Vulnerability details

Impact

In function createAuction(), it has checked for tax tokens when transferring baseToken from seller to contract

// Passes https://github.com/transmissions11/solmate/blob/main/src/utils/SafeTransferLib.sol#L9
// Transfer base tokens to auction contract and check for tax tokens
uint256 balanceBeforeTransfer = ERC20(auctionParams.baseToken).balanceOf(address(this));

SafeTransferLib.safeTransferFrom(
    ERC20(auctionParams.baseToken), msg.sender, address(this), auctionParams.totalBaseAmount
);

uint256 balanceAfterTransfer = ERC20(auctionParams.baseToken).balanceOf(address(this));
if (balanceAfterTransfer - balanceBeforeTransfer != auctionParams.totalBaseAmount) {
    revert UnexpectedBalanceChange();
}

However, in function bid(), similar check is lacking for quoteToken. It will create a problem when seller cannot finalize the auction and bidder cannot refund cause actual balance is less than expected.

Proof of Concept

It did not check for tax tokens in bid() function

SafeTransferLib.safeTransferFrom(ERC20(a.params.quoteToken), msg.sender, address(this), quoteAmount);

emit Bid(
    msg.sender, auctionId, bidIndex, quoteAmount, commitment, pubKey, encryptedMessage, encryptedPrivateKey
);

Tools Used

Manual Review

Recommended Mitigation Steps

Consider adding similar check for pre and after balance of quoteToken like in the function createAuction()

Lines of code

https://github.com/code-423n4/2022-11-size/blob/706a77e585d0852eae6ba0dca73dc73eb37f8fb6/src/SizeSealed.sol#L157

Vulnerability details

Impact

In the function bid(), number of bids is limited to 1000 to avoid out of gas in finalize(). However, it can be abused by attacker to spam bid() function to 1000 bids, block other users from bidding this auction.

Proof of Concept

Maximum number of bids is 1000 in bid():

 uint256 bidIndex = a.bids.length;
// Max of 1000 bids on an auction to prevent DOS
if (bidIndex >= 1000) { // @audit bidder can always spam if merkle proof is not used
    revert InvalidState();
}

Since bids are sealed, price is kept private, so attacker can just place bids that has the price lower than reserveQuotePerBase. It will effectively create the DOS attack without having to buy any baseToken

// Only fill if above reserve price
if (quotePerBase < data.reserveQuotePerBase) continue;

Tools Used

Manual Review

Recommended Mitigation Steps

Actually I cannot find any solution for this issues that do not change the design choice. However considering the design, I recommend to add fee for bidders when they bid(). This way will limit number of bids that attacker can place since they have to lose something now.