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

11 stars 8 forks source link

The first deposit of each pool token will revert #191

Closed c4-bot-5 closed 5 months ago

c4-bot-5 commented 6 months ago

Lines of code

https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/MainHelper.sol#L33-L45

Vulnerability details

when deposit in wise lending,beacuse of the initial number of lendingPoolData[_poolToken].pseudoTotalPool is zero,so it will revert in _calculateShares function

Impact

all token can't deposit in wise lending in unsolely model.

Proof of Concept

Use depositExactAmount function as example:

https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/WiseLending.sol#L469-L474

    function depositExactAmount(
        uint256 _nftId,
        address _poolToken,
        uint256 _amount
    )
        public
        syncPool(_poolToken)
        returns (uint256)
    {
        uint256 shareAmount = _handleDeposit(
            msg.sender,
            _nftId,
            _poolToken,
            _amount
        );

        _safeTransferFrom(
            _poolToken,
            msg.sender,
            address(this),
            _amount
        );

        return shareAmount;
    }

it will call _handleDeposit:

https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/WiseCore.sol#L115-L121

function _handleDeposit(
        address _caller,
        uint256 _nftId,
        address _poolToken,
        uint256 _amount
    )
        internal
        returns (uint256)
    {
        uint256 shareAmount = calculateLendingShares(
            {
                _poolToken: _poolToken,
                _amount: _amount,
                _maxSharePrice: false
            }
        );
...
}

And call calculateLendingShares to calculate share:

https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/MainHelper.sol#L17C14-L45

    function calculateLendingShares(
        address _poolToken,
        uint256 _amount,
        bool _maxSharePrice
    )
        public
        view
        returns (uint256)
    {
        return _calculateShares(
            lendingPoolData[_poolToken].totalDepositShares * _amount,
            lendingPoolData[_poolToken].pseudoTotalPool,
            _maxSharePrice
        );
    }
    function _calculateShares(
        uint256 _product,
        uint256 _pseudo,
        bool _maxSharePrice
    )
        private
        pure
        returns (uint256)
    {
        return _maxSharePrice == true
            ? _product / _pseudo + 1
            : _product / _pseudo - 1;
    }

When pseudoTotalPool is zero, _calculateShares will revert, because divide zero will revert.

So all pool token can't deposit because the initial number of lendingPoolData[_poolToken].pseudoTotalPool is zero.

Tool Used

foundary、vscode

Recommended Mitigation Steps

Add the processing logic of lendingPoolData[_poolToken].pseudoTotalPool == 0

Assessed type

Math

vm06007 commented 6 months ago

This is yet another report without POC or legit proof. Please pay attention to the code base, you can clearly see there's a lot of tests proving you are wrong.

1) All pools are initialized with pseudoTotalPool never 0 it is always 1E3 aka GHOST_AMOUNT 2) Take a look here: https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/PoolManager.sol#L248

You will see that when the pool is created the amount of pseudoTotalPool is NOT equal to 0 https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/WiseLendingDeclaration.sol#L188

So even the first statement and the title of this submission is already wrong

c4-pre-sort commented 5 months ago

GalloDaSballo marked the issue as insufficient quality report

GalloDaSballo commented 5 months ago

I agree with the Sponsor that such a report purposefully removed the POC as obfuscation, these types of reports should have a POC instead of a wall of text

c4-judge commented 5 months ago

trust1995 marked the issue as unsatisfactory: Insufficient proof