SIZE contest - sashik_eth's results

Lines of code

https://github.com/code-423n4/2022-11-size/blob/main/src/SizeSealed.sol#L150-L163

Vulnerability details

Impact

Some ERC20 tokens could have fees for each transfer, they are known as "fee-on-transfer" tokens. While there is a check inside createAuction() function that prevents these tokens from being baseToken, FoT tokens still allowed to be quoteToken which could lead to a problem with accounting correct amount of deposited tokens on contract.

Proof of Concept

Inside bid() function amount of deposited tokens stored from function parameter quoteAmount value, no matter how many tokens are actually transferred to the contract:

src/SizeSealed.sol:150        ebid.quoteAmount = quoteAmount;
    ...
src/SizeSealed.sol:163        SafeTransferLib.safeTransferFrom(ERC20(a.params.quoteToken), msg.sender, address(this), quoteAmount);

So it's a possible scenario:

1. Alice creates an auction with FoT token ABC (with a 5% fee) as quoteToken. She may don't know that ABC has fees on transfer or that fees could be inactive at the moment of auction creation.
2. Bob placed a bid with 100 tokens. While the contract saves a value of 100 in ebid.quoteAmount real contract balance of token ABC is 95 due to the 5% fee.
3. Bob decides to cancel the bid using cancelBid function. While the contract call's safeTransfer() with amount 100 (due to saved value in b.quoteAmount) actual balance is 95 and the transaction revert's. Bob's funds are locked since the contract has lack of tokens ABC.

Recommended Mitigation Steps

Add in bid() function the same check for FoT token as in createAuction() function:

        uint256 balanceBeforeTransfer = ERC20(a.params.quoteToken).balanceOf(address(this));

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

        uint256 balanceAfterTransfer = ERC20(a.params.quoteToken).balanceOf(address(this));
        if (balanceAfterTransfer - balanceBeforeTransfer != quoteAmount) {
            revert UnexpectedBalanceChange();
        }

This would prevent bidders from depositing FoT tokens with activated fees.