sherlock-admin
commented
11 months ago 4 comment(s) were left on this issue during the judging contest.
panprog commented:
invalid because batcher is trusted
n33k commented:
invalid, not convincing without PoC
darkart commented:
Low
polarzero commented:
High. Agree with the comments.
feelereth
high
If batcher is address(0), it should revert instead of relying on the reserve as a fallback. Relying on an untrusted reserve could be dangerous.
Summary
If batcher is address(0), it should revert instead of relying on the reserve as a fallback. Relying on an untrusted reserve could be dangerous.
Vulnerability Detail
The _wrap() and _unwrap() functions are used to wrap/unwrap between USDC and DSU tokens. They first try to use the batcher contract for this, but if the batcher address is 0, they fallback to using the reserve contract directly. The key vulnerable lines are: Link 1 and Link 2 If an attacker controls the reserve contract, they could implement the mint() and redeem() functions to steal funds or manipulate token balances. For example, the attacker could make the mint() function not actually mint any DSU tokens while still returning a success code. This would make the caller believe the wrap succeeded when no tokens were received. Or the redeem() function could burn the input DSU tokens but not send any USDC back. This would let the attacker steal the DSU being unwrapped.
Impact
Code Snippet
https://github.com/sherlock-audit/2023-09-perennial/blob/main/perennial-v2/packages/perennial-extensions/contracts/MultiInvoker.sol#L283-L285 https://github.com/sherlock-audit/2023-09-perennial/blob/main/perennial-v2/packages/perennial-extensions/contracts/MultiInvoker.sol#L297-L299
Tool used
Manual Review
Recommendation
The contract should revert if the batcher address is 0 to prevent falling back to an untrusted reserve