code-423n4 / 2023-09-venus-findings

4 stars 4 forks source link

The immutable numbers of markets in `Prime` contract makes it impossible to add new markets. #491

Closed c4-submissions closed 1 year ago

c4-submissions commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-09-venus/blob/b11d9ef9db8237678567e66759003138f2368d23/contracts/Tokens/Prime/Prime.sol#L288-L309

Vulnerability details

Impact

In the future, the Prime contract reach his maximum limit of markets added inside of it, and this cause a DoS of the Prime::addMarket(). If the Governance want to add a new markets available to the Venus Prime, this is not possible.

Proof of Concept

The Prime contract use a maxLoopsLimit parameter which is declared in the initialize() function to limit the number of potentials markets can be added to the contract. In the actual implementation, no function are implemented to call _setMaxLoopsLimit() to update the maxLoopsLimit. If at one time the maximum length of allMarkets is reached, the addMarket() function will revert because _ensureMaxLoops verify if the maximum length is reached and revert if this is the case.

    function addMarket(address vToken, uint256 supplyMultiplier, uint256 borrowMultiplier) external {

       ...

        _ensureMaxLoops(allMarkets.length);

        emit MarketAdded(vToken, supplyMultiplier, borrowMultiplier);
    }

The addMarket() function will revert all time because there is no way to update the maximum length of allMarkets array, and there is no way to delete a market inside the contract. At this point we can't upgrade the markets configuration. If the Governance want to add a new markets available to the Venus Prime, this is not possible.

Tools Used

Manual Review

Recommended Mitigation Steps

Add a restricted-access function that calls the internal function _setMaxLoopsLimit() to update the maximum length of allMarkets array.

Potentially add a restricted-access function to remove a potential broken/hacked market/VToken pool, it's just a recomendation. You can set one up and give users time to claim their interests before deleting the market. This can help you to have more possible actions to updates the potentials markets the Governance wants to use in Venus Prime, and to respect a decent limit of markets to avoid the for loops in the contract to run out of gas.

Assessed type

Context

c4-pre-sort commented 1 year ago

0xRobocop marked the issue as duplicate of #421

c4-judge commented 1 year ago

fatherGoose1 changed the severity to QA (Quality Assurance)

c4-judge commented 12 months ago

fatherGoose1 marked the issue as grade-b