transfer of Ether without sufficient safety checks in the invoke function

Summary

There is a potential vulnerability related to the transfer of Ether without sufficient safety checks in the invoke function. This vulnerability can lead to Ether being trapped in the contract or misused.

Vulnerability Detail

In the invoke function, there is a line of code that transfers Ether to msg.sender using the transfer function. However, this transfer is executed without sufficient safety checks. It is essential to ensure that Ether transfers are conducted securely, considering possible failure scenarios and reentrancy attacks.

Impact

The lack of safety checks when transferring Ether can have the following impacts:

Loss of Ether: If the transfer fails or is maliciously exploited, Ether can be lost or trapped in the contract.
Vulnerability to Reentrancy Attacks: The lack of proper checks and state updates during Ether transfers can expose the contract to reentrancy attacks.

Code Snippet

Here's the relevant code snippet from the invoke function that transfers Ether to msg.sender:

// Eth must not remain in this contract at rest
payable(msg.sender).transfer(address(this).balance);

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

This code transfers the Ether balance of the contract to msg.sender.

Tool used

Manual Review

Recommendation

To mitigate this vulnerability, it is recommended to implement safety checks and use best practices for handling Ether transfers. Specifically:

Implement checks-effects-interactions pattern to prevent reentrancy attacks.
Ensure that the transfer operation is executed after all state updates to avoid reentrancy vulnerabilities.
Consider using the more robust send or revert pattern for Ether transfers, which provides better control and error handling.
Carefully monitor the gas usage during transfers to prevent out-of-gas issues.

sherlock-audit / 2023-10-perennial-judging

0xVinylDavyl - transfer of Ether without sufficient safety checks in the invoke function #4