Use call instead of transfer on payable addresses

Summary

In Solidity, when transferring Ether, .transfer() and .send() are commonly used. However, they have a limitation: they forward only a stipend of 2300 gas, which isn't enough to execute any code in the recipient contract beyond a simple event emission. Thus, if the recipient is a contract, the transfer may fail unexpectedly.

Vulnerability Detail

To overcome this, Solidity introduced the .call{value: _amount}("") method, which forwards all available gas and can invoke more complex functionality. It's also safer in that it does not revert on failure but instead returns a boolean value to indicate success or failure. Therefore, it is generally a better choice to use .call when transferring Ether to a payable address, with the necessary safety checks implemented to handle potential errors.

Impact

The transfer may fail unexpectedly.

Code Snippet

https://github.com/sherlock-audit/2024-02-perennial-v2-3/blob/main/perennial-v2/packages/perennial-extensions/contracts/MultiInvoker.sol#L162-L163

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

Tool used

Manual Review

Recommendation

Consider replacing instances of .transfer() with .call() such as:

(bool success, ) = msg.sender.call{value: address(this).balance}("");
require(success, "Transfer failed");

Duplicate of #7

sherlock-audit / 2024-02-perennial-v2-3-judging

w42d3n - Use call instead of transfer on payable addresses #34