panprog - `MultiInvoker`, `Manager` and `Account` unexpected reverts in certain conditions due to AAVE reverting on deposits and withdrawals with 0 amount

[sherlock-admin4]

panprog

Medium

MultiInvoker, Manager and Account unexpected reverts in certain conditions due to AAVE reverting on deposits and withdrawals with 0 amount

Summary AAVE v3 pool implementation reverts when trying to deposit or withdraw amount = 0:

  function validateSupply(DataTypes.ReserveCache memory reserveCache, uint256 amount)
    internal
    view
  {
    require(amount != 0, Errors.INVALID_AMOUNT);

AaveV3FiatReserve._update still allows deposits to and withdrawals from AAVE with amount = 0:

    function _update(UFixed18 collateral, UFixed18 target) internal virtual override {
        if (collateral.gt(target))
            aave.withdraw(fiat, UFixed6Lib.from(collateral.sub(target)), address(this));
        if (target.gt(collateral))
            aave.deposit(fiat, UFixed6Lib.from(target.sub(collateral)), address(this), 0);
    }

Note, that when abs(collateral - target) < 1e12, conversion from UFixed18 to UFixed6 will result in amount = 0.

All amounts relevant for this issue are calculated from 6-decimals amounts (unallocated amount is balance of 6-decimals USDC token, allocated amount is balance of 6-decimals aUSDC token, redeemed/deposited amount is UFixed6 in MultiInvoker, Manager and Account), however the target value is calculated as:

        target = unallocated.add(allocated).sub(amount).mul(allocation);

Since unallocated, allocated and amount all will be converted from 6 to 18 decimals - all of them will be divisible by 1e12. But target amount will very likely not be divisible by 1e12. For example, unallocated + allocated - amount = 111e12, allocation = 10% = 0.1e18 = 1e17, then target = 111e12 * 1e17 / 1e18 = 111e11 = 11.1e12.

This means that collateral will almost always be either greater or less, but not equal to target.

Now, the situation when abs(collateral - target) < 1e12 might happen:

• In Account: almost always when user calls withdraw(UFixed6Lib.MAX, true) and Account DSU balance is 0.
• In MultiInvoker and Manager: if the allocated amount (aUSDC) grows by exactly the user's order amount over the time without transactions
• or if admin changes allocation percentage and order's amount matches the difference between collateral and new target exactly

The most likely situation is in the Account: when user tries to withdraw full amount in USDC (setting unwrap = true) (either directly from Account or with signature via Controller), and the account doesn't have any DSU (only USDC), such transactions will almost always revert, denying the user core protocol functionality. This might be time-critical for the user as he might need these funds elsewhere and the unexpected reverts (which will keep happening in consecutive transactions) might make him lose funds from the positions opened or not opened elsewhere.

Much less likely condition for MultiInvoker and Manager: withdrawals with unwrap flag set are vulnerable to this DOS. MultiInvoker or Manager action charges interfaceFee by withdrawing it from user's market balance and if unwrap flag is set, it's converted to USDC via batcher or reserve (if batcher is not set or empty). Since this conversion to USDC will revert in reserve, entire MultiInvoker or Manager transaction will revert as well.

Root Cause AaveV3FiatReserve._update doesn't verify amount passed to aave.deposit and aave.withdraw is not 0. https://github.com/sherlock-audit/2024-08-perennial-v2-update-3/blob/main/emptyset-mono/packages/emptyset-reserve/contracts/reserve/strategy/AaveV3FiatReserve.sol#L66-L69

Internal pre-conditions

1. Reserve uses AAVE strategy
2. Reserve.allocation is not 0
3. (allocated + unallocated - interfaceFee.amount) * reserve.allocation is not divisible by 1e12.

Account: 4a. User's account has some USDC and none DSU 5a. User calls Account.withdraw(UFixed6Lib.MAX, true)

MultiInvoker and Manager: 4b. User has created a MultiInvoker or Manager order with interfaceFee.unwrap or interfaceFee2.unwrap set to true. 5b. The allocated amount in reserve has grown exactly by the amount charged by interface over the time without transactions

External pre-conditions Account: None

MultiInvoker and Manager: Market price is within the order's execution range

Attack Path Happens by itself:

• Account user: all such withdrawal transactions will revert denying user withdrawal of his funds (the funds can still be withdrawn if unwrap is set to false, or exact amount is specified, but if it's another contract, this might not be possible at all).
• MultiInvoker/Manager: user's order can not be executed temporarily due to revert.

Impact Account: User is unable to withdraw his funds and can not allocate them in the other positions, losing funds from liquidation or not benefiting from the position he intended to open.

MultiInvoker / Manager:

• User order is executed at a worse price
• Or position is liquidated (in case the order was a stop-loss and price moved beyound liquidation price)
• Or user order is not executed (in case the order is take profit and the price had moved away from the execution range)

PoC Not needed

Mitigation Convert difference of target and collateral to Fixed6 and compare it to 0, instead of directly comparing target with collateral in AAVE strategy.

[sherlock-admin2]

The protocol team fixed this issue in the following PRs/commits: https://github.com/equilibria-xyz/emptyset-mono/pull/14