sherlock-audit / 2023-09-perennial-judging

0 stars 0 forks source link

feelereth - The _closablePosition calculation is vulnerable to manipulation of pending positions by an attacker. #54

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

feelereth

high

The _closablePosition calculation is vulnerable to manipulation of pending positions by an attacker.

Summary

The _closablePosition calculation depends on mutable pending position data. An attacker could manipulate positions before this calculation to incorrectly increase closable amount.

Vulnerability Detail

_closablePosition iterates through pending positions to calculate the closable amount: Link An attacker could exploit this by:

  1. Calling update() to create a pending position with a high maker amount.
  2. Then calling _closablePosition() before that pending position is settled.
  3. This will make the closable amount higher than it should be, allowing them to redeem more assets than they should be able to.

    Impact

    The attackers is able to redeem more assets than they should be able to.

    Code Snippet

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

    Tool used

Manual Review

Recommendation

_closablePosition calculation should only consider settled/finalized positions, not pending ones.

sherlock-admin commented 1 year ago

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

panprog commented:

invalid because user can't call _closablePosition

n33k commented:

invalid, not convincing without PoC

darkart commented:

Seems that Watson misundertstood the function