denzi_ - setReserveFactor() does not call updateState() before setting new reserveFactor

[sherlock-admin4]

denzi_

High

setReserveFactor() does not call updateState() before setting new reserveFactor

Summary

In PoolFactory.sol, there is an onlyOwner function alled setReserveFactor(uint256 updated)

function setReserveFactor(uint256 updated) external onlyOwner {
    // @audit-issue state should be updated to latest before setting the new reserveFactor
    uint256 old = reserveFactor;
    reserveFactor = updated;
    emit ReserveFactorUpdated(old, updated, msg.sender);
  }

Vulnerability Detail

When the vault owner calls PoolFactory.setReserveFactor(), the function updates the reserve factor. Unfortunately, if the updateState() function is not called before the reserve factor is updated, the updated reserve factor will be applied in the _accrueToTreasury() function when the function is executed next time. This leads to unexpected accruel to treasury as reserveFactor is used to determine how much amount is to be minted towards the reserve.accruedToTreasuryShares()

function _accrueToTreasury(uint256 reserveFactor, DataTypes.ReserveData storage _reserve, DataTypes.ReserveCache memory _cache) internal {
    if (reserveFactor == 0) return;
    AccrueToTreasuryLocalVars memory vars;

    // calculate the total variable debt at moment of the last interaction
    vars.prevtotalDebt = _cache.currDebtShares.rayMul(_cache.currBorrowIndex);

    // calculate the new total variable debt after accumulation of the interest on the index
    vars.currtotalDebt = _cache.currDebtShares.rayMul(_cache.nextBorrowIndex);

    // debt accrued is the sum of the current debt minus the sum of the debt at the last update
    vars.totalDebtAccrued = vars.currtotalDebt - vars.prevtotalDebt;

    vars.amountToMint = vars.totalDebtAccrued.percentMul(reserveFactor);

    if (vars.amountToMint != 0) _reserve.accruedToTreasuryShares += vars.amountToMint.rayDiv(_cache.nextLiquidityIndex).toUint128();
  }

As can be seen, the new reserveFactor will directly apply on the totalDebtAccrued which results in amountToMint. If updateState is not called before updating the reserve factor than this value can be unexpected and different then what it should be. The new values depend on whether the new reserveFactor's value and ignores the accruel upto the point of the previous reserveFactor.

Impact

Wrong calculations will occur when _accrueToTreasury() is called causing amount to be less or more depending on the new value.

Code Snippet

_accrueToTreasury()
setReserveFactor()

Tool used

Manual Review

Recommendation

Update the state before changing the reserve factor

Duplicate of #402

[sherlock-admin3]

1 comment(s) were left on this issue during the judging contest.

Honour commented:

Invalid: see #302