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

4 stars 4 forks source link

Single Oracle Failure Affects All Markets #411

Open c4-submissions opened 11 months ago

c4-submissions commented 11 months ago

Lines of code

https://github.com/code-423n4/2023-09-venus/blob/f60b7110297f7b273288934d648af83897bcf0b2/contracts/Tokens/Prime/Prime.sol#L625-L638 https://github.com/code-423n4/2023-09-venus/blob/f60b7110297f7b273288934d648af83897bcf0b2/contracts/Tokens/Prime/Prime.sol#L211-L219 https://github.com/code-423n4/2023-09-venus/blob/f60b7110297f7b273288934d648af83897bcf0b2/contracts/Tokens/Prime/Prime.sol#L660 https://github.com/code-423n4/2023-09-venus/blob/f60b7110297f7b273288934d648af83897bcf0b2/contracts/Tokens/Prime/Prime.sol#L872-L884

Vulnerability details

Impact

A failure in the oracle for a single market cascades, causing failures across all markets. This obstructs the process of minting prime tokens, inhibits the initialization of markets for users, prevents updating the user's score, and halts operations within the XVSVault.

Proof of Concept

When initializing markets for users and updating their scores, the Prime.updateScores and Prime._initializeMarkets functions are triggered. These loop through all markets and invoke _calculateScore for each one:

    for (uint256 i = 0; i < _allMarkets.length; ) {
        address market = _allMarkets[i];
        accrueInterest(market);

        interests[market][account].rewardIndex = markets[market].rewardIndex;

        uint256 score = _calculateScore(market, account);
        interests[market][account].score = score;
        markets[market].sumOfMembersScore = markets[market].sumOfMembersScore + score;

        unchecked {
            i++;
        }
    }

https://github.com/code-423n4/2023-09-venus/blob/f60b7110297f7b273288934d648af83897bcf0b2/contracts/Tokens/Prime/Prime.sol#L625-L638

    for (uint256 j = 0; j < _allMarkets.length; ) {
        address market = _allMarkets[j];
        _executeBoost(user, market);
        _updateScore(user, market);

        unchecked {
            j++;
        }
    }

https://github.com/code-423n4/2023-09-venus/blob/f60b7110297f7b273288934d648af83897bcf0b2/contracts/Tokens/Prime/Prime.sol#L211-L219

Inside _calculateScore, the function _capitalForScore is called to obtain a user's capital in the market:

    (uint256 capital, , ) = _capitalForScore(xvsBalanceForScore, borrow, supply, market);

https://github.com/code-423n4/2023-09-venus/blob/f60b7110297f7b273288934d648af83897bcf0b2/contracts/Tokens/Prime/Prime.sol#L660

_capitalForScore queries the Resilient Oracle for xvs and the underlying token's prices:

    function _capitalForScore(
        uint256 xvs,
        uint256 borrow,
        uint256 supply,
        address market
    ) internal view returns (uint256, uint256, uint256) {
        address xvsToken = IXVSVault(xvsVault).xvsAddress();

@>      uint256 xvsPrice = oracle.getPrice(xvsToken);
        uint256 borrowCapUSD = (xvsPrice * ((xvs * markets[market].borrowMultiplier) / EXP_SCALE)) / EXP_SCALE;
        uint256 supplyCapUSD = (xvsPrice * ((xvs * markets[market].supplyMultiplier) / EXP_SCALE)) / EXP_SCALE;

@>      uint256 tokenPrice = oracle.getUnderlyingPrice(market)

https://github.com/code-423n4/2023-09-venus/blob/f60b7110297f7b273288934d648af83897bcf0b2/contracts/Tokens/Prime/Prime.sol#L872-L884

If there's an oracle failure in one market, the entire updateScore process crashes, leaving user scores not update, even if other markets are functioning correctly.

And we can see that there are cases when 3 validation in the oracle fails, it triggers the Resilient Oracle to revert the getPrice and getUnderlyingPrice function.

https://github.com/VenusProtocol/oracle/blob/e85cbe2edb4bd94cf2fe9ea9a6183cd1c112d6ff/contracts/ResilientOracle.sol#L363

https://docs-v4.venus.io/risk/resilient-price-oracle#safety-measures

In addition to that, the owner of the Prime contract cannot remove the market to fix the situation.

Furthermore, the Prime contract's owner can't eliminate the troubled market to rectify the issue. Additionally, not refreshing scores for all markets freezes XVSVault deposits and withdrawals, as XVSVault calls Prime.xvsUpdated whenever a user's XVS balance in XVSVault changes.

Tools Used

Manual

Recommended Mitigation Steps

Implement a feature that allows markets to be removed from the Prime contract.

Assessed type

Oracle

c4-pre-sort commented 11 months ago

0xRobocop marked the issue as duplicate of #421

c4-judge commented 10 months ago

fatherGoose1 changed the severity to QA (Quality Assurance)

c4-judge commented 10 months ago

fatherGoose1 marked the issue as grade-b