sherlock-audit / 2024-08-perennial-v2-update-3-judging

4 stars 2 forks source link

bin2chen - when ReserveBase undercollateralized , Manager.orders will not be able to execute #35

Open sherlock-admin3 opened 2 months ago

sherlock-admin3 commented 2 months ago

bin2chen

Medium

when ReserveBase undercollateralized , Manager.orders will not be able to execute

Summary

Manager.sol does not take into account that reserve.redeemPrice may be less than 1:1 The current code, reserve.redeem(amount) followed by a direct transfer of the same USDC, will fail because it results in an insufficient balance and the order will not be triggered successfully

Root Cause

in Manager.sol:219

If balance order.interfaceFee.unwrap=true, need to convert DSU to USDC Use reserve.redeem(amount); But this method, in the case of undercollateralized, is possible to convert less than amount, but the current code implementation logic directly uses amount.

    /// @inheritdoc IReserve
    function redeemPrice() public view returns (UFixed18) {
        // if overcollateralized, cap at 1:1 redemption / if undercollateralized, redeem pro-rata
        return assets().unsafeDiv(dsu.totalSupply()).min(UFixed18Lib.ONE);
    }

    function _unwrapAndWithdaw(address receiver, UFixed18 amount) private {
        reserve.redeem(amount);
        USDC.push(receiver, UFixed6Lib.from(amount));
    }

Internal pre-conditions

No response

External pre-conditions

  1. XXXReserve.sol undercollateralized

Attack Path

  1. alice place TriggerOrder[1] = {price < 123 , interfaceFee.unwrap=true}
  2. XXXReserve.sol undercollateralized , redeemPrice < 1:1
  3. when price < 123 , Meet the order conditions
  4. keeper call executeOrder(TriggerOrder[1]) , but execute fail because revert Insufficient balance

Impact

No response

PoC

No response

Mitigation

    function _unwrapAndWithdaw(address receiver, UFixed18 amount) private {
-       reserve.redeem(amount);
-       USDC.push(receiver, UFixed6Lib.from(amount));
+       USDC.push(receiver, UFixed6Lib.from(reserve.redeem(amount)));
    }
sherlock-admin2 commented 1 month ago

The protocol team fixed this issue in the following PRs/commits: https://github.com/equilibria-xyz/perennial-v2/pull/458