code-423n4 / 2024-07-benddao-findings

5 stars 4 forks source link

wrapNativeTokenInWallet() always reverts on Arbitrum #42

Open c4-bot-1 opened 1 month ago

c4-bot-1 commented 1 month ago

Lines of code

https://github.com/code-423n4/2024-07-benddao/blob/117ef61967d4b318fc65170061c9577e674fffa1/src/libraries/logic/VaultLogic.sol#L789

Vulnerability details

Vulnerability details

VaultLogic.wrapNativeTokenInWallet() is used in a number of places It's mainly used to store msg.value into WETH and then transfer it to msg.sender.

  function wrapNativeTokenInWallet(address wrappedNativeToken, address user, uint256 amount) internal {
    require(amount > 0, Errors.INVALID_AMOUNT);

    IWETH(wrappedNativeToken).deposit{value: amount}();

@>  bool success = IWETH(wrappedNativeToken).transferFrom(address(this), user, amount);
    require(success, Errors.TOKEN_TRANSFER_FAILED);
  }

The token transfer is done using the transferFrom method. This works fine on most chains (Ethereum, Optimism, Polygon, BSC) which use the standard WETH9 contract that handles the case 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;
        }

The problem is that the WETH implementation on Arbitrum uses a different contract, and does not have this src == msg.sender handling.

c4 similar issue: https://github.com/code-423n4/2023-05-chainlink-findings/issues/593

Impact

wrapNativeTokenInWallet() revert , BVault/CrossLending...Contracts that use this method don't execute correctly.

Recommended Mitigation

  function wrapNativeTokenInWallet(address wrappedNativeToken, address user, uint256 amount) internal {
    require(amount > 0, Errors.INVALID_AMOUNT);

    IWETH(wrappedNativeToken).deposit{value: amount}();

-   bool success = IWETH(wrappedNativeToken).transferFrom(address(this), user, amount);
+   bool success = IWETH(wrappedNativeToken).transfer(user, amount);
    require(success, Errors.TOKEN_TRANSFER_FAILED);
  }

Assessed type

Context

c4-judge commented 1 month ago

MarioPoneder marked the issue as primary issue

c4-judge commented 1 month ago

MarioPoneder marked the issue as satisfactory

c4-judge commented 1 month ago

MarioPoneder marked the issue as selected for report

thorseldon commented 1 month ago

Fixed at https://github.com/BendDAO/bend-v2/commit/c7b6150929440505bbbd8a6d15955e56609053f2.