sherlock-audit / 2023-07-perennial-judging

2 stars 1 forks source link

feelereth - the _update function is vulnerable to an attacker draining funds from arbitrary accounts #82

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

feelereth

high

the _update function is vulnerable to an attacker draining funds from arbitrary accounts

Summary

The key issue is that _update does not verify that msg.sender matches the account parameter being updated

Vulnerability Detail

The collateral update happens unconditionally, regardless of who called the function. An attacker could exploit this by:

  1. Calling _update with a victim's address as the account parameter
  2. Setting a negative collateral amount to drain funds This would drain funds from the victim's account even though the msg.sender is the attacker.

Impact

The impact is that an attacker could arbitrarily drain funds from any user's account.

Code Snippet

https://github.com/sherlock-audit/2023-07-perennial/blob/main/perennial-v2/packages/perennial/contracts/Market.sol#L238-L246 https://github.com/sherlock-audit/2023-07-perennial/blob/main/perennial-v2/packages/perennial/contracts/Market.sol#L277

Tool used

Manual Review

Recommendation

_update should add a check that msg.sender == account before updating state

sherlock-admin commented 1 year ago

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

141345 commented:

x

darkart commented:

The function is private

panprog commented:

invalid because update checks the msg.sender in _invariant