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

6 stars 5 forks source link

4b - In `MultiInvoker.sol::_withdraw` Blacklisted USDC addresses can lead to DOS #24

Closed sherlock-admin3 closed 5 months ago

sherlock-admin3 commented 5 months ago

4b

high

In MultiInvoker.sol::_withdraw Blacklisted USDC addresses can lead to DOS

Summary

In MultiInvoker.sol::_withdraw we can see the function unwraps DSU into USDC and pushes USDC amount to account without any checks on the account or any handling of reverts

Vulnerability Detail

The _withdraw function is being used in a lot of functions such as _vaultUpdate and _update to update state automatically for the account. As we know USDC has the blacklist functionality to prevent some addresses from performing transactions, In the case where an address is being blacklisted the transaction will always revert leading to DOS and not able to retrieve funds

Impact

This revert will affect the functionalitiy of the _vaultUpdate, _update and the main function that's the invoke

Code Snippet

_withdraw

    function _withdraw(address account, UFixed6 amount, bool wrap) internal {
        if (wrap) {
            _unwrap(account, UFixed18Lib.from(amount));
        } else {
            DSU.push(account, UFixed18Lib.from(amount));
        }
    }

Tool used

Manual Review

Recommendation

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

Consider adopting a pull method for users to claim their amount.

sherlock-admin4 commented 5 months ago

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

panprog commented:

invalid, nothing can be done with blacklisted user anyway, so it's correct behaviour

takarez commented:

this seem to affect the user thats initiating the transaction and not any other user.

nevillehuang commented 4 months ago

Duplicate of #18