code-423n4 / 2024-05-munchables-findings

3 stars 1 forks source link

WETH compatibility issue on Blast chain #345

Closed howlbot-integration[bot] closed 5 months ago

howlbot-integration[bot] commented 5 months ago

Lines of code

https://github.com/code-423n4/2024-05-munchables/blob/57dff486c3cd905f21b330c2157fe23da2a4807d/src/managers/LockManager.sol#L374-L377

Vulnerability details

Impact

According to provided documentation - it's assumed that the protocol can add more ERC-20 tokens in the future to LockManager, however - WETH is explicitly mentioned as a supported token:

ERC20 used by the protocol  USDB, WETH, (assume we can add more to the future for use in LockManager)

Moreover, it's explicitly stated that the protocol will be deployed on the Blast Mainnet.

The transfer of the token is done using token.transferFrom (line 376). This works fine on most chains (Ethereum, Optimism, Polygon, BSC) which uses the standard WETH9 contract that handles the case when src == msg.sender:

WETH9.sol
        if (src != msg.sender && allowance[src][msg.sender] != uint(- 1)) {
            require(allowance[src][msg.sender] >= wad);
            allowance[src][msg.sender] -= wad;
        }

However, the issue occurs for WETH implementation on Blast, as it uses a different contract and does not have src == msg.sender handling.

The very similar issue was reported during the previous C4 contest and was evaluated as Medium:

Proof of Concept

File: LockManager.sol

        if (_tokenContract != address(0)) {
            IERC20 token = IERC20(_tokenContract);
            token.transferFrom(_tokenOwner, address(this), _quantity);
        }

Above code assumes src == msg.sender handling, which is not a case for WETH on Blast chain.

Tools Used

Manual code review

Recommended Mitigation Steps

It's recommended to change token.transferFrom(_tokenOwner, address(this), _quantity); to token.transfer(address(this), _quantity);

Assessed type

Other