search

sherlock-audit / 2023-07-perennial-judging

2 stars 1 forks source link

YakuzaKiawe - Unable to withdraw funds if `wrap` is true #39

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

YakuzaKiawe

medium

Unable to withdraw funds if wrap is true

YakuzaKiawe

Medium

Unable to withdraw funds if wrap is true

Summary

If the wrap flag is set to true, users cannot withdraw funds from the perennial-v2\packages\perennial-extensions\contracts\MultiInvoker.sol contract.

Vulnerability Detail

The _withdraw function in the perennial-v2\packages\perennial-extensions\contracts\MultiInvoker.sol contract only allows users to withdraw DSU tokens when the wrap flag is set to true. However, it does not allow users to withdraw USDC tokens. This means that users who want to withdraw USDC tokens must first set the wrap flag to false.

This vulnerability is a problem because it prevents users from withdrawing their funds if they need them in USDC. Additionally, it could also lead to users losing their funds if the DSU token price drops significantly.

    /// @notice Push DSU or unwrap DSU to push USDC from this address to `account`
    /// @param account Account to push DSU or USDC to
    /// @param amount Amount to transfer
    /// @param wrap flag to unwrap DSU to USDC
    function _withdraw(address account, UFixed6 amount, bool wrap) internal {
        if (wrap) {
            _unwrap(account, UFixed18Lib.from(amount));
        } else {
            DSU.push(account, UFixed18Lib.from(amount));
        }
    }

This function is used in _liquidate, _vaultUpdate and _update which ultimately mess the working of invoke function in perennial-v2\packages\perennial-extensions\contracts\MultiInvoker.sol

Impact

The impact of this vulnerability could be significant. If users cannot withdraw their funds, they may lose access to their money. Additionally, if the DSU token price drops significantly, users could lose even more money.

Code Snippet

https://github.com/sherlock-audit/2023-07-perennial/blob/main/perennial-v2/packages/perennial-extensions/contracts/MultiInvoker.sol#L256-L266

Tool used

Manual Review

Recommendation

The recommended mitigation for this vulnerability is to add a check to the _withdraw function to see if the wrap flag is set to true. If it is, the function should also withdraw the equivalent amount of USDC tokens. This would allow users to withdraw their funds in either DSU or USDC, depending on their preference.

The following code snippet shows how to implement this mitigation:

if (wrap) {
    USDC.push(account, amount);
    _unwrap(account, UFixed18Lib.from(amount));
}
sherlock-admin commented 1 year ago

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

141345 commented:

d

n33k commented:

user error

darkart commented:

Bad Recommendation

shtesesamoubiq commented:

invalid because it is not sayed why you cannot withdraw USDC when the wrap is true