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

9 stars 6 forks source link

Incorrect unwrapNativeTokenInWallet receiver address #4

Open c4-bot-10 opened 3 months ago

c4-bot-10 commented 3 months ago

Lines of code

https://github.com/code-423n4/2024-07-benddao/blob/main/src/modules/BVault.sol#L74 https://github.com/code-423n4/2024-07-benddao/blob/main/src/modules/CrossLending.sol#L50 https://github.com/code-423n4/2024-07-benddao/blob/main/src/modules/IsolateLending.sol#L51

Vulnerability details

Impact

If the user specifies native tokens as output assets, the protocol will mistakenly try to unwrap wrapped tokens from the msg.sender instead of the receiver address that was specified in the function call.

Proof of Concept

This issue occurs in the following modules:

Users are allowed to specify an output asset when borrowing/withdrawing funds from the protocol. If the output asset is a native token, a wrapped version will be sent to the receiver via the erc20TransferOutLiquidity function, and then the receiver of these tokens must send them to the protocol, which will unwrap and send them back:

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

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

    IWETH(wrappedNativeToken).withdraw(amount);

    safeTransferNativeToken(user, amount);
  }

The problem here is that the wrapped tokens will be sent to the params.receiver address, but the unwrapNativeTokenInWallet function will try to get them from msg.sender, who may not have them in the wallet. As a result withdraw/borrow functionality may fail if msg.sender != receiver:

  function withdrawERC20(
    uint32 poolId,
    address asset,
    uint256 amount,
    address onBehalf,
    address receiver
  ) public whenNotPaused nonReentrant {
    address msgSender = unpackTrailingParamMsgSender();
    DataTypes.PoolStorage storage ps = StorageSlot.getPoolStorage();
    bool isNative;
    if (asset == Constants.NATIVE_TOKEN_ADDRESS) {
      isNative = true;
      asset = ps.wrappedNativeToken;
    }

    SupplyLogic.executeWithdrawERC20(
      InputTypes.ExecuteWithdrawERC20Params({
        msgSender: msgSender,
        poolId: poolId,
        asset: asset,
        amount: amount,
        onBehalf: onBehalf,
>>      receiver: receiver
      })
    );

    if (isNative) {
>>    VaultLogic.unwrapNativeTokenInWallet(asset, msgSender, amount);
    }
  }

Same in crossBorrowERC20 and isolateBorrow functions.

Tools Used

Manual review

Recommended Mitigation Steps

    if (isNative) {
-     VaultLogic.unwrapNativeTokenInWallet(asset, msgSender, amount);
+     VaultLogic.unwrapNativeTokenInWallet(asset, receiver, amount);
    }

Assessed type

Token-Transfer

c4-judge commented 2 months ago

MarioPoneder marked the issue as primary issue

thorseldon commented 2 months ago

Fixed at here: https://github.com/BendDAO/bend-v2/commit/8d2851a0cff5df4b14d6913246730338f5fe8099

c4-judge commented 2 months ago

MarioPoneder marked the issue as satisfactory

c4-judge commented 2 months ago

MarioPoneder marked the issue as selected for report