sherlock-audit / 2023-07-perennial-judging

2 stars 1 forks source link

tives - No validation of the Vault address in invoke #131

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

tives

high

No validation of the Vault address in invoke

Summary

In the invoke function, there is no validation check to ensure that the vault address passed as an argument is a valid Vault contract. This means that the attacker can drain the vault contract.

Vulnerability Detail

function invoke(Invocation[] calldata invocations) external payable {
    for(uint i = 0; i < invocations.length; ++i) {
        Invocation memory invocation = invocations[i];
        } else if (invocation.action == PerennialAction.UPDATE_VAULT) {
                (IVault vault, UFixed6 depositAssets, UFixed6 redeemShares, UFixed6 claimAssets, bool wrap)
                    = abi.decode(invocation.args, (IVault, UFixed6, UFixed6, UFixed6, bool));

                _vaultUpdate(vault, depositAssets, redeemShares, claimAssets, wrap);

// there is no validation about the legitimacy of the Vault contract

next

function _vaultUpdate(
        IVault vault,)
...
        vault.update(msg.sender, depositAssets, redeemShares, claimAssets);

Since this is a custom Vault contract, the update function will successfully return for any claim amount

Then code continues on to withdraw

...
if (!claimAmount.isZero()) {
    _withdraw(msg.sender, claimAmount, wrap);
}

In withdraw, the funds are just pushed to the receiver

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

Impact

Users can drain the DSU from vault

Code Snippet

function _vaultUpdate(
    IVault vault,
    UFixed6 depositAssets,
    UFixed6 redeemShares,
    UFixed6 claimAssets,
    bool wrap
) internal {
... // vault will not revert for custom vault contract
    vault.update(msg.sender, depositAssets, redeemShares, claimAssets);

...
    if (!claimAmount.isZero()) {
        _withdraw(msg.sender, claimAmount, wrap);
    }
}

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

Tool used

Manual Review

Recommendation

Use a Vault whitelist so users cannot use a custom vault.

sherlock-admin commented 1 year ago

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

141345 commented:

x