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

11 stars 8 forks source link

FeeManager.addPoolTokenAddress() function unable to be called due to wrong expected caller #256

Closed c4-bot-7 closed 5 months ago

c4-bot-7 commented 5 months ago

Lines of code

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

Vulnerability details

Impact

According to the dev note

/**
     * @dev Function adding new pool token to pool token list.
     * Called during pool creation and only callable by wiseLending
     * contract.
     */

FeeManager.addPoolTokenAddress() is expected to be called during the pool creation and only by the WiseLending contract.

While, indeed the function is called during pool creation, this call is actually an external call from the PoolManager contract in line260 of PoolManager contract

However, the modifier onlyWiseLending() checks msg.sender == address(WISE_LENDING), if false the call reverts.

Since the call is from the PoolManager, FeeManager.addPoolTokenAddress() would always revert.

Proof of Concept

addPoolTokenAddress() function expected to be called by WiseLending contract only https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/FeeManager/FeeManager.sol#L390C5-L406C6

function addPoolTokenAddress(
        address _poolToken
    )
        external
        onlyWiseLending
    {
        poolTokenAddresses.push(
            _poolToken
        );

        poolTokenAdded[_poolToken] = true;

        emit PoolTokenAdded(
            _poolToken,
            block.timestamp
        );
    }

Call to FeeManager.addPoolTokenAddress() from PoolManager contract https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/PoolManager.sol#L260-L262

FEE_MANAGER.addPoolTokenAddress(
            _params.poolToken
        );

onlyWiseLending() modifier checks if caller is WiseLending contract https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/FeeManager/DeclarationsFeeManager.sol#L181-L183

 modifier onlyWiseLending() {
        _onlyWiseLending();
        _;
    }

https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/FeeManager/DeclarationsFeeManager.sol#L213-L222C6

function _onlyWiseLending()
        private
        view
    {
        if (msg.sender == address(WISE_LENDING)) {
            return;
        }

        revert NotWiseLending();
    }

Tools Used

Manual

Recommended Mitigation Steps

the function should be called by the PoolManager contract rather than WiseLending.

Assessed type

Other

GalloDaSballo commented 5 months ago

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

contract WiseLending is PoolManager {

Looks wrong since WiseLending IS PoolManager

c4-pre-sort commented 5 months ago

GalloDaSballo marked the issue as insufficient quality report

c4-judge commented 5 months ago

trust1995 marked the issue as unsatisfactory: Insufficient proof