Venus Protocol Isolated Pools - J4de's results

Lines of code

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

Vulnerability details

Impact

_getHypotheticalLiquiditySnapshot function is used to calculate the health of all vToken collateral of the user. The problem here is accrueInterest function is not being called to update the interest when all vTokens are counted. As a result, the user's position is healthier than the actual situation.

Proof of Concept

1. The user has borrowed a lot of vTokens and has reached the liquidation threshold
2. However, since the interest has not been updated, the user himself is healthy, and it is still possible to redeem and borrow

Tools Used

manual

Recommended Mitigation Steps

    function _getHypotheticalLiquiditySnapshot(
        address account,
        VToken vTokenModify,
        uint256 redeemTokens,
        uint256 borrowAmount,
        function(VToken) internal view returns (Exp memory) weight
    ) internal view returns (AccountLiquiditySnapshot memory snapshot) {
        // For each asset the account is in
        VToken[] memory assets = accountAssets[account];
        uint256 assetsCount = assets.length;

        for (uint256 i; i < assetsCount; ++i) {
            VToken asset = assets[i];
+           asset.accrueInterest();

Assessed type

Other

Lines of code

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

Vulnerability details

Impact

File: Comptroller.sol
 754         // If collateral factor != 0, fail if price == 0
 755 >>      if (newCollateralFactorMantissa != 0 && oracle.getUnderlyingPrice(address(vToken)) == 0) {
 756             revert PriceError(address(vToken));
 757         }

The price should be updated before getUnderlyingPrice. This can cause setCollateralFactor to succeed if the actual price is 0, or fail if the actual price is not 0.

Proof of Concept

Tools Used

manual

Recommended Mitigation Steps

+       oracle.updatePrice(vToken);
        // If collateral factor != 0, fail if price == 0
        if (newCollateralFactorMantissa != 0 && oracle.getUnderlyingPrice(address(vToken)) == 0) {
            revert PriceError(address(vToken));
        }

Assessed type

Invalid Validation

Lines of code

https://github.com/code-423n4/2023-05-venus/blob/main/contracts/Comptroller.sol#L469-L472

Vulnerability details

Impact

File: Comptroller.sol
 469         uint256 maxClose = mul_ScalarTruncate(Exp({ mantissa: closeFactorMantissa }), borrowBalance);
 470         if (repayAmount > maxClose) {
 471             revert TooMuchRepay();
 472         }

When repayAmount > maxClose, liquidation occurs revert. Attackers can use this mechanism to front-run liquidate a little wei to avoid liquidation.

Proof of Concept

1. Alias liquidates Bob's position
2. Bob front-run liquidate a little
3. Alias's repayAmount > maxClose, then revert

Tools Used

manual

Recommended Mitigation Steps

It is recommended that if the liquidation quantity exceeds the upper limit, the liquidation quantity takes the upper limit value.

Assessed type

DoS

Lines of code

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

Vulnerability details

Impact

File: Shortfall.sol
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         }

The precision of UnderlyingToken may not be 1e18

Proof of Concept

Tools Used

manual

Recommended Mitigation Steps

        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;
+           uint256 usdValue = (priceOracle.getUnderlyingPrice(address(vTokens[i])) * marketBadDebt) / UnderlyingToken.decimals();

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

Assessed type

Decimal

Lines of code

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

Vulnerability details

Impact

Proof of Concept

1. Assume that the current _totalSupply is 0 and the initial initialExchangeRateMantissa is 1:1
2. The attacker mint 1, _totalSupply is 1, underlying.balanceOf(address(this)) is 1
3. The attacker transfers 1 underlying token to this contract, _totalSupply is 1, underlying.balanceOf(address(this)) is 2
4. The user expects mint 1000 and transfers 1000 underlying tokens. At this time, cashPlusBorrowsMinusReserves = 2/1 = 2, and the user gets 1000/2=500 vTokens.

Tools Used

manual

Recommended Mitigation Steps

Maintain totalCash yourself instead of using underlying.balanceOf(address(this))

Assessed type

Other