sherlock-audit / 2023-09-perennial-judging

0 stars 0 forks source link

feelereth - The market collateral could include "trapped" collateral that can't actually be withdrawn. #48

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

feelereth

medium

The market collateral could include "trapped" collateral that can't actually be withdrawn.

Summary

The _collateral function that sums the collateral from each market does not check for "trapped" collateral that can't be withdrawn.

Vulnerability Detail

This is because the _collateral function calculates the total collateral by summing the collateral balances across all registered markets. However, some of this collateral may be "trapped" and unavailable to withdraw in practice. Specifically, the _collateral function doesn't account for unrealized losses on open positions. For example: • The vault has 1 ETH collateral in Market A • It uses this 1 ETH as margin to open a 5 ETH short position in Market A • If the price drops, the short position will have an unrealized loss • Even if the unrealized loss exceeds the 1 ETH collateral, _collateral will still return 1 ETH • So the reported collateral is "trapped" behind the underwater position

Impact

• Over-rebalancing positions based on inflated collateral estimates • Allowing large redemptions that exceed real available collateral • Social losses if collateral estimates are too high

Code Snippet

https://github.com/sherlock-audit/2023-09-perennial/blob/main/perennial-v2/packages/perennial-vault/contracts/Vault.sol#L569-L573

Tool used

Manual Review

Recommendation

Have _collateral incorporate unrealized PnL into its estimates

sherlock-admin commented 1 year ago

3 comment(s) were left on this issue during the judging contest.

panprog commented:

invalid because collateral is settled each valid oracle and there is no unrealized profit or loss, everything is immediately realized to collateral upon settlement

n33k commented:

invalid, not convincing without PoC

darkart commented:

Thats because it shouldnt be traped