search

sherlock-audit / 2023-07-perennial-judging

2 stars 1 forks source link

feelereth - Code can lock funds by approving the batcher and reserve without a way to revoke approval #95

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

feelereth

high

Code can lock funds by approving the batcher and reserve without a way to revoke approval

Summary

The code does approve the batcher and reserve to transfer tokens via approve(), and there is no way to revoke that approval. This can potentially lock funds if the batcher or reserve is malicious

Vulnerability Detail

There is no way to revoke the approval after it has been granted. The standard ERC20 approve() method only allows setting the approval amount, not revoking it.

Impact

If either the batcher or reserve is malicious, they could potentially drain all the DSU and USDC tokens from this contract, locking the funds.

Code Snippet

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

Tool used

Manual Review

Recommendation

The contract could use the ERC20 increaseAllowance() and decreaseAllowance() methods to carefully control the approval amounts rather than approving the maximum amount. It could also revoke approvals when no longer needed by setting the approval to 0:

sherlock-admin commented 1 year ago

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

141345 commented:

x

panprog commented:

invalid because owner is trusted