Skip to content

Latest commit

 

History

History
66 lines (36 loc) · 2.76 KB

053.md

File metadata and controls

66 lines (36 loc) · 2.76 KB

Cheesy Pebble Kookaburra

Medium

Controller::withdrawWithSignature may not work due to wrong assumption of 1 DSU = 1 USDC

Summary

This issue was reported in a previous audit 2024 here and also in a previous audit 2023 here. However, in the collateral Account the same issue still exists with assuming 1:1 unwrapping of DSU into USDC when reserve.redeem(amount) is called.

As a consequence, Controller::withdrawWithSignature() and ControllerIncentivized::withdrawWithSignature() may not work due to this issue.

Root Cause

In Account::withdraw(), USDC may be unwrapped from DSU to facilitate withdrawal by calling reserve.redeem(amount) in Account.sol line 103.

However the redeem price may be below 1 in the reserve implementation:

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);
}

Account::withdraw() then calls USDC.push(owner, pushAmount) with pushAmount being the amount. However since unwrapping DSU to USDC may not result in 1:1 conversion, this may revert when trying to send more USDC to the owner than the contract has.

Code snippets:

https://github.com/sherlock-audit/2025-01-perennial-v2-4-update/blob/2381b47e69b17fe892b1d1d0f467c358cf950143/perennial-v2/packages/periphery/contracts/CollateralAccounts/Controller.sol#L228

https://github.com/sherlock-audit/2025-01-perennial-v2-4-update/blob/2381b47e69b17fe892b1d1d0f467c358cf950143/perennial-v2/packages/periphery/contracts/CollateralAccounts/Controller_Incentivized.sol#L165

https://github.com/sherlock-audit/2025-01-perennial-v2-4-update/blob/2381b47e69b17fe892b1d1d0f467c358cf950143/perennial-v2/packages/periphery/contracts/CollateralAccounts/Account.sol#L75-L85

Internal Pre-conditions

No response

External Pre-conditions

No response

Attack Path

No response

Impact

Controller::withdrawWithSignature() and ControllerIncentivized::withdrawWithSignature() may not work due to this issue.

PoC

No response

Mitigation

Consider adjusting Account::withdraw() to transfer the difference between balance before and balance after, similar as in MultiInvoker.sol line 398:

// Account::withdraw()
83    UFixed6 pushAmount = amount.eq(UFixed6Lib.MAX) ? USDC.balanceOf() : USDC.balanceOf().sub(usdcBalance);