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

11 stars 8 forks source link

[H-1] Repetitive _safeTransfer call leading to loss of funds in protocol #214

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/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/PowerFarms/PendlePowerFarm/PendlePowerFarm.sol#L157-L179 https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/WiseLending.sol#L902-L933

Vulnerability details

Description:

withdrawAmount is been withdrawn twice in PendlePowerFarm.sol::_manuallyWithdrawShares internal function. It makes a _safeTransfer with a value to the msg.sender in withdrawExactShares and a _safeTransfer is being called in the same code block right after.

Impact:

This would Lead to a loss of funds to the protocol since users can now withdraw from their deposited shares twice in a single transaction.

Tools Used

Manual Review

Proof Of Concept:

     function _manuallyWithdrawShares(
        uint256 _nftId,
        uint256 _withdrawShares
    )
        internal
    {
        uint256 withdrawAmount = WISE_LENDING.cashoutAmount(
            PENDLE_CHILD,
            _withdrawShares
        );

        // @audit-
        withdrawAmount = WISE_LENDING.withdrawExactShares(
            _nftId,
            PENDLE_CHILD,
            _withdrawShares
        );

        // @audit-
        _safeTransfer(
            PENDLE_CHILD,
            msg.sender,
            withdrawAmount
        );
    }

Recommended Mitigation Steps:

Change the code logic to allow for only a single withdrawAmount in the _manuallyWithdrawShares internal function

    function _manuallyWithdrawShares(
        uint256 _nftId,
        uint256 _withdrawShares
    )
        internal
    {
        uint256 withdrawAmount = WISE_LENDING.cashoutAmount(
            PENDLE_CHILD,
            _withdrawShares
        );

        withdrawAmount = WISE_LENDING.withdrawExactShares(
            _nftId,
            PENDLE_CHILD,
            _withdrawShares
        );

-        _safeTransfer(
-            PENDLE_CHILD,
-            msg.sender,
-            withdrawAmount
-        );
    }

Assessed type

ETH-Transfer

GalloDaSballo commented 5 months ago

Seems invalid as WiseLending will call

https://github.com/code-423n4/2024-02-wise-lending/blob/1240a22a3bbffc13d5f8ae6300ef45de5edc7c19/contracts/WiseLending.sol#L702-L705

        _sendValue(
            msg.sender,
            withdrawAmount
        );

Where msg.sender is PendlePowerFarm

Submission also lacks a POC

c4-pre-sort commented 5 months ago

GalloDaSballo marked the issue as insufficient quality report

vm06007 commented 5 months ago

Should be invalid.

c4-judge commented 5 months ago

trust1995 marked the issue as unsatisfactory: Invalid