sherlock-audit / 2023-09-perennial-judging

0 stars 0 forks source link

feelereth - _socialize() function is vulnerable to being gamed by attackers #53

Closed sherlock-admin2 closed 1 year ago

sherlock-admin2 commented 1 year ago

feelereth

high

_socialize() function is vulnerable to being gamed by attackers

Summary

The _socialize() function could be gamed by attackers by carefully controlling deposit/redeem amounts.

Vulnerability Detail

The _socialize() function calculates the claimAmount that the user will receive after depositing/redeeming, by taking into account social losses and fees. It does this by calculating the totalCollateral as: Link1. And then calculating the claimAmount as: Link2 This means the claimAmount depends on the totalCollateral in the system.

An attacker could exploit this by:

  1. Making a large deposit to increase the totalCollateral significantly.
  2. Then making a small redeem to trigger the _socialize() function.
  3. Because the totalCollateral is now larger due to their deposit, but the global.assets has remained the same, their claimAmount will be a larger % of their original claimAssets than it should be.
  4. This allows them to extract more assets than they deposited, at the expense of other users.

    Impact

Code Snippet

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

Tool used

Manual Review

Recommendation

The totalCollateral should be calculated based on the state at the beginning of the transaction, before any deposits/redeems. Or probably by changing the socialization math to use a time-weighted average collateral level over a period rather than a snapshot.

sherlock-admin commented 1 year ago

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

panprog commented:

invalid because watson didn't fully understand the process and it will be correct in the scenario presented

n33k commented:

invalid, not convincing without PoC