code-423n4 / 2022-01-sandclock-findings

0 stars 0 forks source link

[WP-N7] `Vault.sol` Tokens with fee on transfer are not supported #163

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

WatchPug

Vulnerability details

There are ERC20 tokens that charge fee for every transfer() or transferFrom().

Vault.sol#_transferAndCheckUnderlying() requires that the received amount is the same as the transfer amount, otherwise, it will revert at L587.

https://github.com/code-423n4/2022-01-sandclock/blob/a90ad3824955327597be00bb0bd183a9c228a4fb/sandclock/contracts/Vault.sol#L580-L591

function _transferAndCheckUnderlying(address _from, uint256 _amount)
    internal
{
    uint256 balanceBefore = totalUnderlying();
    underlying.safeTransferFrom(_from, address(this), _amount);
    uint256 balanceAfter = totalUnderlying();

    require(
        balanceAfter == balanceBefore + _amount,
        "Vault: amount received does not match params"
    );
}
naps62 commented 2 years ago

As stated in a prior issue (can't find it right now) we don't intend to support tokens where this would be a problem. Our intent so far is to support USDC, UST and DAI

dmvt commented 2 years ago

duplicate of #55