sherlock-audit / 2023-01-optimism-judging

22 stars 8 forks source link

0xWeiss - [M-05] Incompatibility with fee on transfer tokens #293

Closed github-actions[bot] closed 1 year ago

github-actions[bot] commented 1 year ago

0xWeiss

medium

[M-05] Incompatibility with fee on transfer tokens

Summary

Fee on transfer tokens are not supported.

Vulnerability Detail

In contract: https://github.com/sherlock-audit/2023-01-optimism/blob/main/optimism/packages/contracts-bedrock/contracts/universal/StandardBridge.sol?plain=1#L414

In lines:

        IERC20(_localToken).safeTransferFrom(_from, address(this), _amount);
        deposits[_localToken][_remoteToken] = deposits[_localToken][_remoteToken] + _amount;

you can see that the accountability of the funds is done before the transfer. This is not compatible with tokens that have a fee on transfer because eventually the bridge will receive less funds that the user sent.

Impact

Optimism will be unable to birdge the tokens to the receiver because there won't be enough funds in the bridge contract.

Code Snippet

/home/sherlock/optimism/packages/contracts-bedrock/contracts/universal/StandardBridge.sol?plain=1

Tool used

Manual Review

Recommendation

 Use balanceAfter - balanceBefore:

uint256 balanceBefore = deflationaryToken.balanceOf(address(this));
deflationaryToken.safeTransferFrom(msg.sender, address(this), takerPrice);
uint256 balanceAfter = deflationaryToken.balanceOf(address(this));
premium = (balanceAfter - balanceBefore);
 deposits[_localToken][_remoteToken] = deposits[_localToken][_remoteToken] + premium;
rcstanciu commented 1 year ago

Comment from Optimism


Description: Weird tokens

Reason: Deflationary and weird tokens are explicitly not supported by the Standard Bridge per a comment in the code.