code-423n4 / 2024-02-wise-lending-findings

11 stars 8 forks source link

The amount of ETH given to the user in `AaveHub.sol#borrowExactAmountETH` function is not accurate. #223

Closed c4-bot-8 closed 5 months ago

c4-bot-8 commented 5 months ago

Lines of code

https://github.com/code-423n4/2024-02-wise-lending/blob/main/contracts/WrapperHub/AaveHub.sol#L393-L427 https://github.com/code-423n4/2024-02-wise-lending/blob/main/contracts/WrapperHub/AaveHelper.sol#L141-L163

Vulnerability details

Impact

When borrowing ETH, the profit generated from Aave is not lent to the user, so the profit is locked in the contract.

Proof of Concept

AaveHub.sol#borrowExactAmountETH function is as follows.

    function borrowExactAmountETH(
        uint256 _nftId,
        uint256 _borrowAmount
    )
        external
        nonReentrant
        returns (uint256)
    {
        _checkOwner(
            _nftId
        );

405:    uint256 borrowShares = _wrapBorrowExactAmount(
406:        _nftId,
407:        WETH_ADDRESS,
408:        address(this),
409:        _borrowAmount
410:    );
411:
412:    _unwrapETH(
413:        _borrowAmount
414:    );
415:
416:    _sendValue(
417:        msg.sender,
418:        _borrowAmount
419:    );

        emit IsBorrowAave(
            _nftId,
            block.timestamp
        );

        return borrowShares;
    }

Because _wrapBorrowExactAmount is called in L405, borrowshares becomes larger than _borrowAmount. However, in L412-L416, only _borrowAmount is given to the user, so the difference between borrowShares and _borrowAmount remains as an expectation. Meanwhile, AaveHelper.sol#_wrapBorrowExactAmount function is as follows.

    function _wrapBorrowExactAmount(
        uint256 _nftId,
        address _underlyingAsset,
        address _underlyingAssetRecipient,
        uint256 _borrowAmount
    )
        internal
        returns (uint256)
    {
        uint256 borrowShares = WISE_LENDING.borrowOnBehalfExactAmount(
            _nftId,
            aaveTokenAddress[_underlyingAsset],
            _borrowAmount
        );

156:    AAVE.withdraw(
156:        _underlyingAsset,
158:        _borrowAmount,
159:        _underlyingAssetRecipient
        );

        return borrowShares;
    }

Here, _underlyingAssetRecipient is address(this) as seen in L408, so the profit generated from Aave is locked in the contract.

Tools Used

Manual Review

Recommended Mitigation Steps

The profits generated from Aave must be transferred to the user.

Assessed type

Token-Transfer

GalloDaSballo commented 5 months ago

This submission should have spent more time explaining what's going on

c4-pre-sort commented 5 months ago

GalloDaSballo marked the issue as insufficient quality report

trust1995 commented 5 months ago

High without coded / worded POC = insufficient proof.

c4-judge commented 5 months ago

trust1995 marked the issue as unsatisfactory: Insufficient proof