sherlock-audit / 2024-02-perennial-v2-3-judging

6 stars 5 forks source link

4b - In `Vault.sol::update` USDC blacklists can lead to locked up funds #18

Closed sherlock-admin3 closed 5 months ago

sherlock-admin3 commented 5 months ago

4b

high

In Vault.sol::update USDC blacklists can lead to locked up funds

Summary

When pushing assets to the receiver the protocol doesn't check whether receiver is blacklisted or not & this can lead to lock up of funds.

Vulnerability Detail

In Vault.sol::update the function updates the accounts position by socializing, updating positions & managing. in the manage code block of the same function, we can see that the claimAmount is being pushed or sent to the receiver(i.e msg.sender). And there's no implementation to check whether receiver is blacklisted or not, as a result this can lead to a revert causing the lock up of funds.

Impact

This can lead to lock up of funds

Code Snippet

https://github.com/sherlock-audit/2024-02-perennial-v2-3/blob/main/perennial-v2/packages/perennial-vault/contracts/Vault.sol#L283-L321

Tool used

Manual Review

Recommendation

The current implementation uses a "push" approach where reward tokens are sent to the recipient during every update, which introduces additional attack surfaces that the attackers can exploit.

Consider adopting a method for users to claim their rewards instead so that the transfer of reward tokens is disconnected from the updating of reward balances.

sherlock-admin2 commented 5 months ago

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

panprog commented:

invalid, there is nothing can be done about blacklisted user anyway, so the behaviour is correct

takarez commented:

this seem to be for msg.sender and will notr cause problem for other users.

nevillehuang commented 4 months ago

Invalid based on sherlock rules:

  1. User Blacklist: User getting blacklisted by a token/contract causing harm only to themselves is not a valid medium/high.