feelereth - The _wrap() function in MultiInvoker.sol allows specifying a different receiver address than the original depositor. This could lead to potential confusion about where the wrapped DSU ends up.

[sherlock-admin2]

feelereth

high

The _wrap() function in MultiInvoker.sol allows specifying a different receiver address than the original depositor. This could lead to potential confusion about where the wrapped DSU ends up.

Summary

It assumes wrapped DSU will be sent back to the deposit address. However, the _wrap() function allows specifying a different receiver. This could lead to potential confusion on where funds ended up

Vulnerability Detail

There is potential for confusion around where wrapped DSU tokens end up when using the _wrap() function in the MultiInvoker contract.

1. The _wrap() function allows specifying a receiver address different from msg.sender to receive the newly wrapped DSU tokens: Link1
2. By default, _wrap() is called from the _deposit() function, which passes address(this) as the receiver: Link2
3. This means newly wrapped DSU tokens will be sent to the MultiInvoker contract by default. However, since _wrap() allows specifying any receiver, a developer could call it directly and pass a different address. This would send the new DSU tokens to that address instead of back to the msg.sender.

Impact

• Original depositor may not receive expected wrapped DSU
• Assets could be sent to incorrect or malicious accounts
• Difficult to track where deposits ended up

  Code Snippet

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

Tool used

Manual Review

Recommendation

Only allow wrapping to the original msg.sender:. The _wrap logic could be separated into two functions, one for wrapping to the depositor and one for arbitrary accounts if needed. But removing the ability to arbitrarily redirect funds prevents confusion.

[sherlock-admin]

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

panprog commented:

invalid because there is no problem with this flaw, "potential confusion" is not a valid issue

n33k commented:

invalid, not convincing without PoC

darkart commented:

Work as intended

polarzero commented:

Invalid. Although it does indeed seem misleading to allow such various untracked operations, it does not seem to be a significant enough issue to be considered as medium/high severity.