Accumulated fee logic can prevent withdrawals
Description
The accumulatedFee
storage variable represents the accumulated fee, charged per strategy in deposited queued withdrawals that should be sent to the treasury when the admin calls withdrawEther
.
However, it is increased when a queued withdrawal is deposited, not when it is completed:
function depositWithQueuedWithdrawal(IStrategyManager.QueuedWithdrawal calldata _queuedWithdrawal, address _referral) external whenNotPaused nonReentrant returns (uint256) {
// ... snip ...
/// handle fee
uint256 feeAmount = _queuedWithdrawal.strategies.length * getFeeAmount();
require(amount >= feeAmount + BALANCE_OFFSET, "less than the fee amount");
amount -= feeAmount;
accumulatedFee += uint128(feeAmount);
This means that, at the specific time that accumulatedFee
is increased, the amount that it represents has not actually entered the Liquifier's balance yet.
So, when the number of deposited withdrawal strategies spikes, due to market conditions or an attack, when an admin calls withdrawEther
,
function withdrawEther() external onlyAdmin {
require(address(this).balance >= accumulatedFee, "not enough balance");
the "not enough balance"
check can be made to fail because accumulatedFee
can be arbitrarily increased by depositors.
Impact
Admins are unable to directly call withdrawEther
until they complete withdrawals, which may not be possible until time passes.
The impact is limited because, since the contract is upgradable, the funds are not locked. Also, a self-destruct payment (to bypass the receive-function sender whitelist) can be sent to the contract to increase its balance atomically right before withdrawEther
is called and then the same amount repaid from the liquidity pool.
Recommendations
A simple solution would be to remove this require statement and add logic such that if the balance is less, all of the withdrawn balance goes to the treasury and accumulatedFee
decreases by the amount sent out.
Alternatively, ensure that the accounting of this fee is always solvent by, instead of making the fee payable in ETH, minting the fee amount of eETH to the treasury, either at deposit time or at withdrawEther
time. Logically, this is equivalent to the user paying the fee out of the minted eETH they received from their deposit.
Remediation
This issue has been acknowledged by Gadze Finance SEZC, and fixes were implemented in the following commits: