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

11 stars 8 forks source link

[H-3] Share's health factor not checked leading to wrong positions health state. #244

Closed c4-bot-1 closed 5 months ago

c4-bot-1 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/InterfaceHub/IWiseLending.sol#L306-L312 https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/WiseLending.sol#L67-L90

Vulnerability details

Description:

Function Interface IWiseLending::withdrawExactShares does not apply essential modifiers and checks for syncPool() and healthStateCheck(). hence, functions inheriting the interface like PendlePowerFarm::_manuallyWithdrawShares are not checked to know if positions are healthy or not and might lead to wrong values in ascertaining positions that should be liquidated.

Impact:

Break in core contract logic in asscertaining the health factor of positions

Tools Used

Manual Review

Proof Of Concept:

    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
        );
    }

Recommended Mitigation Steps:

Create an interface in the IWiseLending with an external function for

    function healthStateCheck() external

Add additional checks to check for the health State of positions in PendlePowerFarm::_manuallyWithdrawShares using the _healthStateCheck()

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

+       withdrawAmount = WISE_LENDING.healthStateCheck(nftId)

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

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

Assessed type

Context

c4-pre-sort commented 5 months ago

GalloDaSballo marked the issue as insufficient quality report

GalloDaSballo commented 5 months ago

Missing impact in context

c4-pre-sort commented 5 months ago

GalloDaSballo marked the issue as primary issue

c4-judge commented 5 months ago

trust1995 marked the issue as unsatisfactory: Insufficient proof

trust1995 commented 5 months ago

Insufficient proof of impact is valid for scrapping.