Closed sherlock-admin3 closed 5 months ago
2 comment(s) were left on this issue during the judging contest.
z3s commented:
Invalid; Sending remaining ETH is the intended functionality.
takarez commented:
but at the end of the function, ether will not remain and work as intended.
1337web3
medium
Medium 02 Lost Funds
Summary
The
_invoke
function within the codebase is designed to execute a batch of invocations for an account based on a provided list of actions. However, a potential vulnerability has been identified in the handling of Ether (ETH) sent to the contract, as detailed below.Vulnerability Detail
The
_invoke
function, despite a comment indicating that ETH should not remain in the contract at rest, currently transfers any incoming ETH directly to a specified account using theAddress.sendValue(payable(account), address(this).balance)
method. This design introduces a risk of potential loss of ETH, as it indiscriminately forwards any ETH sent to the contract to the specified account without adequate validation or consideration.https://github.com/sherlock-audit/2024-05-kwenta-x-perennial-integration-update/blob/main/perennial-v2/packages/perennial-extensions/contracts/MultiInvoker.sol#L190
Impact
This vulnerability exposes the contract to potential financial loss, as any ETH sent to the contract will be transferred out without appropriate safeguards. Additionally, it may undermine the trust and confidence of users interacting with the contract.
Code Snippet
Tool used
Manual Review
Recommendation
It's imperative to handle ETH sent to contracts securely and transparently. To mitigate this vulnerability, consider implementing a mechanism to reject or handle unexpected ETH transfers appropriately. Another way of mitigating this issue is a
withdraw
function set with anonlyOwner
modifier. It will give the owner the possibility to withdraw the funds sent to the contract.