Venus Protocol Isolated Pools - Brenzee's results

Lines of code

https://github.com/code-423n4/2023-05-venus/blob/723001cf7bc0f37aba26fb385ec1a60135f24fe3/contracts/WhitePaperInterestRateModel.sol#L17

Vulnerability details

Impact

blockPerYear value in WhitePaperInterestRateModel.sol contract is incorrect for protocol since Venus will deploy these contracts to Binance Smart Chain.

Proof of Concept

In WhitePaperInterestRateModel.sol contract blockPerYear constant value is 2102400.

    /**
     * @notice The approximate number of blocks per year that is assumed by the interest rate model
     */
    uint256 public constant blocksPerYear = 2102400; // @audit - BSC block time is 3 seconds, so this is not correct.

This is incorrect, because there are more blocks per year on Binance Smart Chain.

Proof

BSC Block time is 3 seconds

60 (seconds) * 60 (minutes) * 24 (day) * 365 (days) = 31536000 (seconds per year) 31536000 / 3 = 10512000 (blocks per year on BSC)

Tools Used

Manual Review

Recommended Mitigation Steps

Change blocksPerYear constant value to 10512000

    uint256 public constant blocksPerYear = 10512000;

Assessed type

Other

Lines of code

https://github.com/code-423n4/2023-05-venus/blob/723001cf7bc0f37aba26fb385ec1a60135f24fe3/contracts/Shortfall/Shortfall.sol#L403-L406

Vulnerability details

Impact

In Shortfall._startAuction function riskFundBalance (asset balance) is compared with incentivizedRiskFundBalance (USD Value), which could cause the if statement to initiate incorrect code.

Proof of Concept

In Shortfall._startAuction function incentivizedRiskFundBalance is calculated from poolBadDebt and poolBadDebt is calculated in a for loop, where all VToken.badDebt gets calculated accumulated as USD value

Shortfall.sol L:389-399

        for (uint256 i; i < marketsCount; ++i) {
            uint256 marketBadDebt = vTokens[i].badDebt();

            priceOracle.updatePrice(address(vTokens[i]));
            uint256 usdValue = (priceOracle.getUnderlyingPrice(address(vTokens[i])) * marketBadDebt) / 1e18;

            poolBadDebt = poolBadDebt + usdValue;
            auction.markets[i] = vTokens[i];
            auction.marketDebt[vTokens[i]] = marketBadDebt;
            marketsDebt[i] = marketBadDebt;
        }

After that riskFundBalance gets fetched from riskFund.poolReserves function and incentivizedRiskFundBalance is calculated from poolBadDebt and both of these values are compared in the if statement.

Shortfall.sol L:403-406

        uint256 riskFundBalance = riskFund.poolReserves(comptroller);
        uint256 remainingRiskFundBalance = riskFundBalance;
        uint256 incentivizedRiskFundBalance = poolBadDebt + ((poolBadDebt * incentiveBps) / MAX_BPS);
        if (incentivizedRiskFundBalance >= riskFundBalance) {

If we check RiskFund.swapPoolsAssets function, we see that poolReserves is the asset amount NOT the USD amount. RiskFund.sol L:174-175

            uint256 swappedTokens = _swapAsset(vToken, comptroller, amountsOutMin[i], paths[i]);
            poolReserves[comptroller] = poolReserves[comptroller] + swappedTokens;

This means that the asset amount gets compared to the USD amount, which is incorrect and the _startAuction function would not operate in the intended way.

Tools Used

Manual Review

Recommended Mitigation Steps

Make sure to calculate riskFundBalance as USD value and compare it to incentivizedRiskFundBalance instead of riskFundBalance itself.

Assessed type

Invalid Validation