Venus Prime - joaovwfreire's results

Lines of code

https://github.com/code-423n4/2023-09-venus/blob/main/contracts/Tokens/Prime/Prime.sol#L970-L979 https://github.com/code-423n4/2023-09-venus/blob/main/contracts/Tokens/Prime/Prime.sol#L40

Vulnerability details

Impact

At the _incomeDistributionYearly function, the Prime contract utilizes a BLOCKS_PER_YEAR constant defined on deployment that will account for the amount of blocks per year at the deployed chain.

function _incomeDistributionYearly(address vToken) internal view returns (uint256 amount) {
        uint256 totalIncomePerBlockFromMarket = _incomePerBlock(vToken);
        uint256 incomePerBlockForDistributionFromMarket = (totalIncomePerBlockFromMarket * _distributionPercentage()) /
            IProtocolShareReserve(protocolShareReserve).MAX_PERCENT();
        amount = BLOCKS_PER_YEAR * incomePerBlockForDistributionFromMarket;

        uint256 totalIncomePerBlockFromPLP = IPrimeLiquidityProvider(primeLiquidityProvider)
            .getEffectiveDistributionSpeed(_getUnderlying(vToken));
        amount += BLOCKS_PER_YEAR * totalIncomePerBlockFromPLP;
    }

This value is then utilized for calculation of the APR. The issue is there is no deterministic way to predict the amount of blocks a year will have due to the nature of block validation and leap years.

The calculated APR value will have different imprecision values depending on the chain: at chains with higher block rates, the impact should be bigger, as shown at the proof of concept. Even though the function that utilizes this variable is view-only (does not modify the chain's state), the risk lies on false claims that a front-end could display and other protocols could attempt to integrate this function creating risks if the purpose is composability. Moreover, the Prime system is made to create value for the XVS token and by displaying wrong values, taking part on such a system may become less attractive.

Proof of Concept

This source code is not in scope but illustrates that the team has once implemented a constant for the amount of blocks per year at the BNB chain at the @venusprotocol/isolated-pools package:

/// @dev The approximate number of blocks per year that is assumed by the interest rate model
uint256 constant BLOCKS_PER_YEAR = 10_512_000;

This constant is based on the assumption that the BNB chain will have 3 second block times - therefore running 28800 blocks per day -> 10512000 blocks per year.

We can draw the assumption that the team will utilize constants to determine the amounts of block per year. In a production setting, however, the amount of blocks per year is not a constant, as shown by the following data:

Amount of blocks that has paased (measured till October 1st) on the Ethereum, BNB and Arbitrum:

From Ethereum Block Count and Rewards Chart:

From BNB Smart Chain Block Count and Rewards Chart:

From Arbitrum Smart Chain Block Count and Rewards Chart:

As shown above, the block amounts are very hard to predict and will lead to considerable imprecision on the APR calculations for staking positions on the smaller end, even on BNB, which has a relatively stable block rate. Consider the graph from the documentation and the following reduction:

The graph shows that positions close to the 1000 XVS range, will earn around 2.5-3% APR. On a not so bad case, consider BNB in 2021. The imprecision would lead to 4.7 days not accounted for, approximately 1,28% of the year. This means the view function would display to users 1.28% more APR than the actual value. On a worst case scenario, consider a contract deployed on Arbitrum in the beginning of 2023. The imprecision would lead to extra 191196 blocks accounted for per day. That would lead to a decrease of over 2/3 on the APR displayed to users. That would be a 2.5-3% APR turned to 0.75-1%.

Tools Used

Manual review

Recommended Mitigation Steps

The team should make the BLOCKS_PER_YEAR variable dynamic, so that it can be updated as the chain evolves. Additionally, a disclaimer should be added to the documentation, warning users that the APR displayed is an estimate and may not reflect the actual value.

Assessed type

Math