The provided code has an issue where it assumes the entire position is liquidatable, when only a portion may be closable.

Summary

It assumes the entire position is liquidatable. However, the closable amount returned from _latest() may only be a portion of the overall position.

Vulnerability Detail

The _liquidate() function calls _liquidationFee() to get the liquidation fee and closable amount. _liquidationFee() in turn calls _latest() to get the latest position and closable amount.

However, _latest() only returns the portion of the overall position that is currently closable based on pending settlements. This closable amount may be less than the full position magnitude.

When _liquidate() is called, it passes the account's entire pending position from market.pendingPositions() to market.update(), not just the closable amount returned from _liquidationFee().

This means it could try to close more of the position than is actually closable, which would fail.

For example:

Account has 10 DSU overall position
Only 5 DSU are closable due to pending settlement
_latest() returns 5 DSU closable amount
_liquidate() tries to close the entire 10 DSU position
This will fail as only 5 DSU can be closed
Impact

The liquidation will fail if it tries to close more than the closable amount. Only the closable portion will be closed.

Code Snippet

https://github.com/sherlock-audit/2023-09-perennial/blob/main/perennial-v2/packages/perennial-extensions/contracts/MultiInvoker.sol#L226-L238

Tool used

Manual Review

Recommendation

_liquidate() should close only the closable amount returned from _liquidationFee(), not the full pending position. This ensures it only closes the portion of the position that is actually closable.

sherlock-audit / 2023-09-perennial-judging

feelereth - The provided code has an issue where it assumes the entire position is liquidatable, when only a portion may be closable. #35