Venus Prime - ast3ros's results

Lines of code

https://github.com/code-423n4/2023-09-venus/blob/edc2212c77c8a419bd49a05ec1e2556405095922/contracts/Tokens/Prime/libs/Scores.sol#L15 https://github.com/code-423n4/2023-09-venus/blob/f60b7110297f7b273288934d648af83897bcf0b2/contracts/Tokens/Prime/Prime.sol#L661 https://github.com/code-423n4/2023-09-venus/blob/f60b7110297f7b273288934d648af83897bcf0b2/contracts/Tokens/Prime/Prime.sol#L651 https://github.com/code-423n4/2023-09-venus/blob/f60b7110297f7b273288934d648af83897bcf0b2/contracts/Tokens/Prime/Prime.sol#L654

Vulnerability details

Impact

Due to an inaccurate capital computation, the score will be wrong, leading to imprecise reward allocations for users.

Proof of Concept

The score calculation for each user is based on a capital input standardized to 18 decimals:

    /**
    * @notice Calculate a membership score given some amount of `xvs` and `capital`, along
    *  with some 𝝰 = `alphaNumerator` / `alphaDenominator`.
    * @param xvs amount of xvs (xvs, 1e18 decimal places)
    * @param capital amount of capital (1e18 decimal places)
    * @param alphaNumerator alpha param numerator
    * @param alphaDenominator alpha param denominator
    * @return membership score with 1e18 decimal places
    *
    * @dev 𝝰 must be in the range [0, 1]
    */
    function calculateScore(
        uint256 xvs,
        uint256 capital,
        uint256 alphaNumerator,
        uint256 alphaDenominator
    ) internal pure returns (uint256) {}

https://github.com/code-423n4/2023-09-venus/blob/edc2212c77c8a419bd49a05ec1e2556405095922/contracts/Tokens/Prime/libs/Scores.sol#L15

Capital is normalized to 18 decimals using the formula:

    capital = capital * (10 ** (18 - vToken.decimals()));

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

The flaw lies in assuming the capital is represented in vToken's decimals. In reality, the capital reflects the underlying asset's decimals.

    uint256 borrow = vToken.borrowBalanceStored(user); // return value is in underlying asset's decimal
    uint256 supply = (exchangeRate * balanceOfAccount) / EXP_SCALE; // return value is in underlying asset's decimal 

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

To correctly standardize the capital to 18 decimals, it should utilize:

    capital = capital * (10 ** (18 - underlying.decimals()));

Tools Used

Manual

Recommended Mitigation Steps

Modify the capital normalization to 18 decimals using the correct formula:

    function _calculateScore(address market, address user) internal returns (uint256) {
        uint256 xvsBalanceForScore = _xvsBalanceForScore(_xvsBalanceOfUser(user));

        IVToken vToken = IVToken(market);
        uint256 borrow = vToken.borrowBalanceStored(user);
        uint256 exchangeRate = vToken.exchangeRateStored();
        uint256 balanceOfAccount = vToken.balanceOf(user);
        uint256 supply = (exchangeRate * balanceOfAccount) / EXP_SCALE;

        address xvsToken = IXVSVault(xvsVault).xvsAddress();
        oracle.updateAssetPrice(xvsToken);
        oracle.updatePrice(market);

        (uint256 capital, , ) = _capitalForScore(xvsBalanceForScore, borrow, supply, market);
-       capital = capital * (10 ** (18 - vToken.decimals()));
+       address underlying = _getUnderlying(vToken);
+       capital = capital * (10 ** (18 - underlying.decimals()));

        return Scores.calculateScore(xvsBalanceForScore, capital, alphaNumerator, alphaDenominator);
    }

Assessed type

Math

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