Venus Protocol Isolated Pools - BPZ's results

Lines of code

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

Vulnerability details

Borrow rate and supply rate are incorrect due to incorrect blocksPerYear value

This protocol is going to be deployed to BNB chain so its needed to calculate blocksPerYear relevant to BNB Smart Chain. But here it has taken blocksPerYear relevant to the ethereum chain.

Proof of Concept

  17  uint256 public constant blocksPerYear = 2102400;

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

Here it should be blocksPerYear = 10512000

Due to this incorrect value its caused a incorrect values for baseRatePerBlock & multiplierPerBlock

  37   baseRatePerBlock = baseRatePerYear / blocksPerYear;
  38    multiplierPerBlock = multiplierPerYear / blocksPerYear;
  

https://github.com/code-423n4/2023-05-venus/blob/main/contracts/WhitePaperInterestRateModel.sol#{L37,L38}

Its also affected to getBorrowRate & getSupplyRate functions as shown below.

  56    return ((ur * multiplierPerBlock) / BASE) + baseRatePerBlock;

https://github.com/code-423n4/2023-05-venus/blob/main/contracts/WhitePaperInterestRateModel.sol#L56

  74    uint256 borrowRate = getBorrowRate(cash, borrows, reserves);

https://github.com/code-423n4/2023-05-venus/blob/main/contracts/WhitePaperInterestRateModel.sol#L74

Tools Used

Manual Auditing

Recommended Mitigation Steps

Use correct value for blocksPerYear relevant to binance smart chain. So it should be

blocksPerYear = 10512000

Correct value is in BaseJumpRateModelV2.sol file https://github.com/code-423n4/2023-05-venus/blob/main/contracts/BaseJumpRateModelV2.sol#L23

Assessed type

Math

Lines of code

https://github.com/code-423n4/2023-05-venus/blob/main/contracts/Shortfall/Shortfall.sol#L393

Vulnerability details

Hardcoded decimal places caused incorrect usdValue for poolBadDebt

When calculating the poolBadDebt in terms of USD value , first take the marketBadDebt in terms of underlying token then it is multiplied by the corresponding price finally its divided by the corresponding decimal places. But here it has been divided by the 1e18.

Proof of Concept

389   for (uint256 i; i < marketsCount; ++i) {
390        uint256 marketBadDebt = vTokens[i].badDebt();
391
392        priceOracle.updatePrice(address(vTokens[i]));
393        uint256 usdValue = (priceOracle.getUnderlyingPrice(address(vTokens[i])) * marketBadDebt) / 1e18;
394
395        poolBadDebt = poolBadDebt + usdValue;
396        auction.markets[i] = vTokens[i];
397        auction.marketDebt[vTokens[i]] = marketBadDebt;
398        marketsDebt[i] = marketBadDebt;
399    }

https://github.com/code-423n4/2023-05-venus/blob/main/contracts/Shortfall/Shortfall.sol#L393

If we consider the WBTC here

Consider the current price of WBTC = 30000 USD (UnderlyingPrice)

badDebt = 2 WBTC token = 2 x 10^8 (Decimals of WBTC = 10 ^ 8)

So usdValue = 30000 x 2 x 10^ 8 / 1e18

This value round off to zero.

But the correct value should be = 30000 x 2 x 10^ 8 / 10^ 8 = 60000 USD

Due to this miscalculation poolBadDebt is less compared with actual poobBadDebts it should be. So its unable to start auction even though there is enough poolBadDebts exists to start the auction. (Since its showing less poolBaddebts).

https://github.com/code-423n4/2023-05-venus/blob/main/contracts/Shortfall/Shortfall.sol#L401

Tools Used

Manual Auditing

Recommended Mitigation Steps

It should be divided by the corresponding decimals for each underlying tokens.

  392      priceOracle.updatePrice(address(vTokens[i]));
  ++       IERC20Upgradeable erc20 = IERC20Upgradeable(address(vTokens[i].underlying()));
  393      uint256 usdValue = (priceOracle.getUnderlyingPrice(address(vTokens[i])) * marketBadDebt) / erc20.decimals() ;

Assessed type

Decimal

Lines of code

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

Vulnerability details

riskFund.poolReserves return the token amount & its compaired with the USD value {units are not same}

poolReserves in RiskFund.sol returns the token amount. But its compaired with the incentivizedRiskFundBalance which is in USD value.

Proof of Concept

 403              uint256 riskFundBalance = riskFund.poolReserves(comptroller);
                  uint256 remainingRiskFundBalance = riskFundBalance;
                  uint256 incentivizedRiskFundBalance = poolBadDebt + ((poolBadDebt * incentiveBps) / MAX_BPS);
                  if (incentivizedRiskFundBalance >= riskFundBalance) {
                      auction.startBidBps =
                          (MAX_BPS * MAX_BPS * remainingRiskFundBalance) /
                          (poolBadDebt * (MAX_BPS + incentiveBps));
                      remainingRiskFundBalance = 0;
                      auction.auctionType = AuctionType.LARGE_POOL_DEBT;
                      

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

Here incentivizedRiskFundBalance in USD & riskFundBalance is token amount. Due to this its always incentivizedRiskFundBalance < riskFundBalance. Hence else part executed.

412            } else {
                      uint256 maxSeizeableRiskFundBalance = incentivizedRiskFundBalance;

                      remainingRiskFundBalance = remainingRiskFundBalance - maxSeizeableRiskFundBalance;
                      auction.auctionType = AuctionType.LARGE_RISK_FUND;
                      auction.startBidBps = MAX_BPS;
                  }

                  auction.seizedRiskFund = riskFundBalance - remainingRiskFundBalance;
                  auction.startBlock = block.number;
                  auction.status = AuctionStatus.STARTED;
                  auction.highestBidder = address(0);
                  

https://github.com/code-423n4/2023-05-venus/blob/main/contracts/Shortfall/Shortfall.sol#L412

Here auction.seizedRiskFund = riskFundBalance - remainingRiskFundBalance;

                        = riskFundBalance - ( remainingRiskFundBalance - maxSeizeableRiskFundBalance) 
                        
                        = remainingRiskFundBalance - remainingRiskFundBalance + maxSeizeableRiskFundBalance
                        
                        = maxSeizeableRiskFundBalance 
                        
                        = incentivizedRiskFundBalance
                        
                        = poolBadDebt + ((poolBadDebt * incentiveBps) / MAX_BPS)

This final value is in USD . This is the amount that going to be send after auction finished.

241           if (auction.auctionType == AuctionType.LARGE_POOL_DEBT) {
                    riskFundBidAmount = auction.seizedRiskFund;
                } else {
                    riskFundBidAmount = (auction.seizedRiskFund * auction.highestBidBps) / MAX_BPS;
                }

                uint256 transferredAmount = riskFund.transferReserveForAuction(comptroller, riskFundBidAmount);
                IERC20Upgradeable(convertibleBaseAsset).safeTransfer(auction.highestBidder, riskFundBidAmount);

https://github.com/code-423n4/2023-05-venus/blob/main/contracts/Shortfall/Shortfall.sol#L248

But on the other hand bidder needs to pay in tokens

  188                } else {
                            if (auction.highestBidder != address(0)) {
                                  erc20.safeTransfer(auction.highestBidder, auction.marketDebt[auction.markets[i]]);
                              }

                              erc20.safeTransferFrom(msg.sender, address(this), auction.marketDebt[auction.markets[i]]);
                             }

https://github.com/code-423n4/2023-05-venus/blob/main/contracts/Shortfall/Shortfall.sol#L190

So Finally bidder get small amount of tokens compared to his spend for the bidding.

Tools Used

Manual Auditing

Recommended Mitigation Steps

keep the incentivizedRiskFundBalance in terms of token value not USD value.

Assessed type

Math