sherlock-audit / 2023-07-perennial-judging

2 stars 1 forks source link

cryptphi - No user supplied input validation in MultiInvoker.invoke() can lead to loss of funds #148

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

cryptphi

high

No user supplied input validation in MultiInvoker.invoke() can lead to loss of funds

Summary

Arbitrary invocation input can lead to loss of funds in MultiInvoker contract

Vulnerability Detail

The MultiInvoker.invoke() allows any user to supply Invocation[] calldata invocations as input for the invoke() payable call. When a user supplies input such that invocation.action == PerennialAction.UPDATE_POSITION , then the contract attemps to update the user supplied market in https://github.com/sherlock-audit/2023-07-perennial/blob/main/perennial-v2/packages/perennial-extensions/contracts/MultiInvoker.sol#L170-L185

Since the arguments in MultiInvoker._update() are user controlled, the malicious user can ensure that value of collateral.sign() == -1 which would then make a withdrawal of the supplied amount to the caller.

Impact

Loss of funds

Code Snippet

if (invocation.action == PerennialAction.UPDATE_POSITION) {
                (
                    IMarket market,
                    UFixed6 newMaker,
                    UFixed6 newLong,
                    UFixed6 newShort,
                    Fixed6 collateral,
                    bool wrap
                ) = abi.decode(invocation.args, (IMarket, UFixed6, UFixed6, UFixed6, Fixed6, bool));

                _update(market, newMaker, newLong, newShort, collateral, wrap);

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

Tool used

Manual Review

Recommendation

Ensure user supplied inputs are validated

sherlock-admin commented 1 year ago

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

141345 commented:

d