sherlock-audit / 2023-05-ironbank-judging

2 stars 2 forks source link

devScrooge - Interest is not accrued in crucial parts #409

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

devScrooge

high

Interest is not accrued in crucial parts

Summary

Interest is not accrued when calling to certain functions that set new values to important parameters that change the "behaviour" of IronBank

Vulnerability Detail

The MarketConfigurator.sol implements some functions that are used for the owner to change some important parameters related with the markets and the IronBank.sol contract.

In some of these functions, such as adjustMarketReserveFactorthe interest is accrued before changing the MarketReserverFactor parameter:

function adjustMarketReserveFactor(address market, uint16 reserveFactor) external onlyOwner {
        DataTypes.MarketConfig memory config = getMarketConfiguration(market);
        require(config.isListed, "not listed");
        require(reserveFactor <= MAX_RESERVE_FACTOR, "invalid reserve factor");

        // Accrue interests before changing IRM.
        ironBank.accrueInterest(market);

        config.reserveFactor = reserveFactor;
        ironBank.setMarketConfiguration(market, config);

        emit MarketReserveFactorSet(market, reserveFactor);
    }
// Accrue interests before changing IRM.
ironBank.accrueInterest(market);

It is also correctly done on the changeMarketInterestRateModel function:

 function changeMarketInterestRateModel(address market, address interestRateModelAddress) external onlyOwner {
        DataTypes.MarketConfig memory config = getMarketConfiguration(market);
        require(config.isListed, "not listed");

        // Accrue interests before changing IRM.
        ironBank.accrueInterest(market);

        config.interestRateModelAddress = interestRateModelAddress;
        ironBank.setMarketConfiguration(market, config);

        emit MarketInterestRateModelSet(market, interestRateModelAddress);
    }
// Accrue interests before changing IRM.
ironBank.accrueInterest(market);

But there are other functions that change important market parameters but does not call to .accrueInterest() before doing it. These functions are:

  1. adjustMarketCollateralFactor:

    function adjustMarketCollateralFactor(address market, uint16 collateralFactor) external onlyOwner {
        DataTypes.MarketConfig memory config = getMarketConfiguration(market);
        require(config.isListed, "not listed");
        if (collateralFactor > 0) {
            require(collateralFactor <= MAX_COLLATERAL_FACTOR, "invalid collateral factor");
            require(
                collateralFactor <= config.liquidationThreshold, "collateral factor larger than liquidation threshold"
            );
        }
    
        config.collateralFactor = collateralFactor;
        ironBank.setMarketConfiguration(market, config);
    
        emit MarketCollateralFactorSet(market, collateralFactor);
    }
  2. adjustMarketLiquidationThreshold:

    function adjustMarketLiquidationThreshold(address market, uint16 liquidationThreshold) external onlyOwner {
        DataTypes.MarketConfig memory config = getMarketConfiguration(market);
        require(config.isListed, "not listed");
        if (liquidationThreshold > 0) {
            require(liquidationThreshold <= MAX_LIQUIDATION_THRESHOLD, "invalid liquidation threshold");
            require(
                liquidationThreshold >= config.collateralFactor, "liquidation threshold smaller than collateral factor"
            );
            require(
                uint256(liquidationThreshold) * uint256(config.liquidationBonus) / FACTOR_SCALE
                    <= MAX_LIQUIDATION_THRESHOLD_X_BONUS,
                "liquidation threshold * liquidation bonus larger than 100%"
            );
        } else {
            require(config.collateralFactor == 0, "collateral factor not zero");
        }
    
        config.liquidationThreshold = liquidationThreshold;
        ironBank.setMarketConfiguration(market, config);
    
        emit MarketLiquidationThresholdSet(market, liquidationThreshold);
    }
  3. adjustMarketLiquidationBonus:

    function adjustMarketLiquidationBonus(address market, uint16 liquidationBonus) external onlyOwner {
        DataTypes.MarketConfig memory config = getMarketConfiguration(market);
        require(config.isListed, "not listed");
        if (liquidationBonus > 0) {
            require(
                liquidationBonus > MIN_LIQUIDATION_BONUS && liquidationBonus <= MAX_LIQUIDATION_BONUS,
                "invalid liquidation bonus"
            );
            require(
                uint256(config.liquidationThreshold) * uint256(liquidationBonus) / FACTOR_SCALE
                    <= MAX_LIQUIDATION_THRESHOLD_X_BONUS,
                "liquidation threshold * liquidation bonus larger than 100%"
            );
        } else {
            require(
                config.collateralFactor == 0 && config.liquidationThreshold == 0,
                "collateral factor or liquidation threshold not zero"
            );
        }
    
        config.liquidationBonus = liquidationBonus;
        ironBank.setMarketConfiguration(market, config);
    
        emit MarketLiquidationBonusSet(market, liquidationBonus);
    }

Impact

Changing market variables that affects on how the earned interest is calculated without first calling to a .accrueInterest() lead to a wrong calculation of the earned interest.

Code Snippet

https://github.com/sherlock-audit/2023-05-ironbank/blob/main/ib-v2/src/protocol/pool/MarketConfigurator.sol#L165-L179 https://github.com/sherlock-audit/2023-05-ironbank/blob/main/ib-v2/src/protocol/pool/MarketConfigurator.sol#L205-L226 https://github.com/sherlock-audit/2023-05-ironbank/blob/main/ib-v2/src/protocol/pool/MarketConfigurator.sol#L233-L257

Tool used

Manual Review

Recommendation

Call to .accrueInterest() before always changing some of the market variables that affect to the interest calculation.