Venus Protocol Isolated Pools - SaeedAlipoor01988's results

Lines of code

https://github.com/code-423n4/2023-05-venus/blob/8be784ed9752b80e6f1b8b781e2e6251748d0d7e/contracts/BaseJumpRateModelV2.sol#L23 https://github.com/code-423n4/2023-05-venus/blob/8be784ed9752b80e6f1b8b781e2e6251748d0d7e/contracts/BaseJumpRateModelV2.sol#L172 https://github.com/code-423n4/2023-05-venus/blob/8be784ed9752b80e6f1b8b781e2e6251748d0d7e/contracts/BaseJumpRateModelV2.sol#L112

Vulnerability details

Impact

The variables baseRatePerBlock and multiplierPerBlock and jumpMultiplierPerBlock can have wrong values.

Proof of Concept

For today, 5/12/2023, Binance Smart Chain Blocks Per Day is at a current level of 28766.00, up from 28753.00 yesterday and up from 28721.00 one year ago. This is a change of 0.05% from yesterday and 0.16% from one year ago. https://ycharts.com/indicators/binance_smart_chain_blocks_per_day

As you can see, blocks per year can get changed and this change can be a large number. but blocksPerYear hardcoded by 10512000 value in the BaseJumpRateModelV2 contract and there is not any function to update it.

If we assume that blocks per day are a fixed value at 28760.00, then 365 days * 28760.00 is equal to 10497400 and not 10512000.

function _updateJumpRateModel(
    uint256 baseRatePerYear,
    uint256 multiplierPerYear,
    uint256 jumpMultiplierPerYear,
    uint256 kink_
) internal {
    baseRatePerBlock = baseRatePerYear / blocksPerYear;
    multiplierPerBlock = (multiplierPerYear * BASE) / (blocksPerYear * kink_);
    jumpMultiplierPerBlock = jumpMultiplierPerYear / blocksPerYear;
    kink = kink_;

    emit NewInterestParams(baseRatePerBlock, multiplierPerBlock, jumpMultiplierPerBlock, kink);
}

blocksPerYear directly affects values of baseRatePerBlock and multiplierPerBlock and jumpMultiplierPerBlock in the BaseJumpRateModelV2 contract. BaseJumpRateModelV2 contract is using these values to return borrowRate and supplyRate.

 */
function _getBorrowRate(
    uint256 cash,
    uint256 borrows,
    uint256 reserves
) internal view returns (uint256) {
    uint256 util = utilizationRate(cash, borrows, reserves);

    if (util <= kink) {
        return ((util * multiplierPerBlock) / BASE) + baseRatePerBlock;
    } else {
        uint256 normalRate = ((kink * multiplierPerBlock) / BASE) + baseRatePerBlock;
        uint256 excessUtil;
        unchecked {
            excessUtil = util - kink;
        }
        return ((excessUtil * jumpMultiplierPerBlock) / BASE) + normalRate;
    }
}

 */
function getSupplyRate(
    uint256 cash,
    uint256 borrows,
    uint256 reserves,
    uint256 reserveFactorMantissa
) public view virtual override returns (uint256) {
    uint256 oneMinusReserveFactor = BASE - reserveFactorMantissa;
    uint256 borrowRate = _getBorrowRate(cash, borrows, reserves);
    uint256 rateToPool = (borrowRate * oneMinusReserveFactor) / BASE;
    return (utilizationRate(cash, borrows, reserves) * rateToPool) / BASE;
}

Tools Used

Manually

Recommended Mitigation Steps

add function to change blocksPerYear value.

Assessed type

Other

Lines of code

https://github.com/code-423n4/2023-05-venus/blob/8be784ed9752b80e6f1b8b781e2e6251748d0d7e/contracts/BaseJumpRateModelV2.sol#L131

Vulnerability details

Impact

The BaseJumpRateModelV2.sol#L131.utilizationRate() function can return value above 1 and not between [0, BASE].

Proof of Concept

In The BaseJumpRateModelV2.sol#L131.utilizationRate() function, cash and borrows and reserves values gets used to calculate utilization rate between between [0, 1e18]. reserves is currently unused but it will be used in the future.

 */
function utilizationRate(
    uint256 cash,
    uint256 borrows,
    uint256 reserves
) public pure returns (uint256) {
    // Utilization rate is 0 when there are no borrows
    if (borrows == 0) {
        return 0;
    }

    return (borrows * BASE) / (cash + borrows - reserves);
}

If Borrow value is 0, then function will return 0. but in this function the scenario where the value of reserves exceeds cash is not handled. the system does not guarantee that reserves never exceeds cash. the reserves grow automatically over time, so it might be difficult to avoid this entirely.

If reserves > cash (and borrows + cash - reserves > 0), the formula for utilizationRate above gives a utilization rate above 1.

Tools Used

Manually

Recommended Mitigation Steps

Make the utilization rate computation return 1 if reserves > cash.

Assessed type

Math

Lines of code

https://github.com/code-423n4/2023-05-venus/blob/8be784ed9752b80e6f1b8b781e2e6251748d0d7e/contracts/VToken.sol#L696 https://github.com/code-423n4/2023-05-venus/blob/8be784ed9752b80e6f1b8b781e2e6251748d0d7e/contracts/VToken.sol#L372

Vulnerability details

Impact

If borrowRateMantissa <= borrowRateMaxMantissa, then call to the VToken.sol#L678.accrueInterest() will get reverted and the entire contract may halt and be unrecoverable.

Proof of Concept

Before executing most methods, the VToken contracts update interest accumulated on borrows via the method VToken.sol#L678.accrueInterest(). This method uses the contract’s interest rate model to calculate the borrow interest rate. If the calculated value is above borrowRateMaxMantissa, the method will revert.

 */Calculate the current borrow interest rate
    uint256 borrowRateMantissa = interestRateModel.getBorrowRate(cashPrior, borrowsPrior, reservesPrior);
    require(borrowRateMantissa <= borrowRateMaxMantissa, "borrow rate is absurdly high");

If this method reverts, the entire contract may halt and be unrecoverable. If this method reverts, the entire contract may halt and be unrecoverable. The only ways to change the values used to calculate this interest rate lie in methods that must first call accrueInterest(). In this case, those methods would fail.

One other potential avenue for recovery exists, the Owner role may update the interest rate calculation contract via VToken.sol#L369.setInterestRateModel() function.

 */
function setInterestRateModel(InterestRateModel newInterestRateModel) external override {
    _checkAccessAllowed("setInterestRateModel(address)");

    accrueInterest();
    _setInterestRateModelFresh(newInterestRateModel);
}

However, this method also calls accrueInterest before completing the upgrade, so it would fail as well.

Tools Used

Manually

Recommended Mitigation Steps

prevent utilization rate calculations from returning anything above borrowRateMaxMantissa or Remove the accrueInterest from VToken.sol#L369.setInterestRateModel() function.

Assessed type

Math