Fees could be dripped in any state
Description
According to Cozy's design, fees could be dripped when SafetyModules's state is SafetyModuleState.ACTIVE.
In FeesHandler, the function _dripFeesFromReservePool is used to accumulate dripped fees from the reserve pool. This function is called from dripFees, dripFeesFromReservePool, claimFees, _executeReserveDeposit, and redeem.
But only two functions — dripFees and dripFeesFromReservePool — check state. The others do not check that SafetyModules's state is SafetyModuleState.ACTIVE.
This causes fees to drip in any state which means that the fees may be overcharged.
function dripFees() public override {
! if (safetyModuleState != SafetyModuleState.ACTIVE) return;
IDripModel dripModel_ = cozySafetyModuleManager.getFeeDripModel(ISafetyModule(address(this)));
uint256 numReserveAssets_ = reservePools.length;
for (uint8 i = 0; i < numReserveAssets_; i++) {
! _dripFeesFromReservePool(reservePools[i], dripModel_);
}
}
function dripFeesFromReservePool(uint8 reservePoolId_) external {
! if (safetyModuleState != SafetyModuleState.ACTIVE) return;
IDripModel dripModel_ = cozySafetyModuleManager.getFeeDripModel(ISafetyModule(address(this)));
! _dripFeesFromReservePool(reservePools[reservePoolId_], dripModel_);
}
function claimFees(address owner_) external {
// ...
uint256 numReservePools_ = reservePools.length;
for (uint8 i = 0; i < numReservePools_; i++) {
ReservePool storage reservePool_ = reservePools[i];
! _dripFeesFromReservePool(reservePool_, dripModel_);
// ...
}
}
! function _dripFeesFromReservePool(ReservePool storage reservePool_, IDripModel dripModel_) internal override {
uint256 drippedFromDepositAmount_ = _getNextDripAmount(
reservePool_.depositAmount - reservePool_.pendingWithdrawalsAmount, dripModel_, reservePool_.lastFeesDripTime
);
if (drippedFromDepositAmount_ > 0) {
reservePool_.feeAmount += drippedFromDepositAmount_;
reservePool_.depositAmount -= drippedFromDepositAmount_;
}
reservePool_.lastFeesDripTime = uint128(block.timestamp);
}Impact
The function _dripFeesFromReservePool subtracts fees from reservePool_.depositAmount. This means a decrease in the asset price of depositing users.
A malicious user could call claimFees repeatedly no matter SafetyModule's state. This causes unexpected fees to be charged from SafetyModule.
Recommendations
We recommend adding the check for SafetyModuleState.ACTIVE to all points before calling _dripFeesFromReservePool. This could prevent SafetyModule from dripping fees when the state is not active.