SIZE contest - horsefacts's results

Lines of code

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

Vulnerability details

The createAuction function checks for fee-on-transfer tokens and reverts if fewer tokens are transferred than expected:

SizeSealed#createAuction:

        // 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, it does not account for interest-bearing or positive rebasing tokens, like Aave aTokens or Lido stETH.

Impact: Since auctions are based on the stored totalBaseAmount, any additional balance accrued following the original transfer will be locked in the auction contract.

Recommendation: This is probably not best solved at the smart contract level. Instead, ensure users are aware of this incompatibility and recommend appropriate wrappers when available (like wstETH for Lido stETH).

Test Case:

First, add a rebase function to MockERC20 that increases an account's balance by 1%:

function rebase(address account) external {
  _mint(account, (balanceOf[account] * 100) / 10000);
}

Then, use it in the following test case:

function testPositiveRebasingToken() public {
  (uint256 beforeQuote, uint256 beforeBase) = seller.balances();
  seller.createAuction(
    baseToSell,
    reserveQuotePerBase,
    minimumBidQuote,
    startTime,
    endTime,
    unlockTime,
    unlockEnd,
    cliffPercent
  );
  baseToken.rebase(address(auction));
  seller.cancelAuction();
  (uint256 afterQuote, uint256 afterBase) = seller.balances();

  // Excess balance remains in the auction contract
  assertEq(baseToken.balanceOf(address(auction)), 0.1 ether);
}

Lines of code

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

Vulnerability details

Solmate's SafeTransfer helper library does not verify that the transferred token exists (i.e. that it is a real contract with deployed bytecode) and instead delegates this responsibility to the caller. A comment in createAuction mentions this important caveat:

createAuction:

        // 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));

However, although createAuction checks that the baseToken exists, there is no corresponding check for the quoteToken in SizeSealed.sol. If an auction is accidentally or intentionally created with a nonexistent quoteToken address, the transfer inside the bid function will silently succeed, allowing bidders to place bids without payment:

bid:

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

Impact:

• If a seller makes a typo in the quote token address, bidders can place bids and win the auction without payment.
• If a malicious third party tricks a seller into using an invalid token address, bidders can place bids and win the auction without payment.

Severity: Medium. High impact, since an auction's baseToken amount may be won without payment. Low likelihood, since it's in the seller's interest to verify their auction parameters, and sellers can cancel a misconfigured auction. (At least, as long as they notice!)

Recommendation: Ensure the quoteToken contract exists (check that quoteToken.code.length != 0) before storing it in createAuction.

Test Case:

function testInvalidQuoteToken() public {
  seller = new MockSeller(address(auction), MockERC20(address(0)), baseToken);
  uint256 aid = seller.createAuction(
    baseToSell,
    reserveQuotePerBase,
    minimumBidQuote,
    startTime,
    endTime,
    unlockTime,
    unlockEnd,
    cliffPercent
  );
  bidder1.setAuctionId(aid);
  bidder1.bidOnAuctionWithSalt(1 ether, 5e6, "hello");
  uint256[] memory bidIndices = new uint[](1);
  bidIndices[0] = 0;

  // Auction can be created, finished, and finalized with an invalid quoteToken
  vm.warp(endTime + 1);
  seller.finalize(bidIndices, 1 ether, 5e6);
}