AdamSzymanski - Use of Deprecated .transfer() Method in Ether Transfers

[sherlock-admin2]

AdamSzymanski

Medium

Use of Deprecated .transfer() Method in Ether Transfers

Summary

The _transferEtherRewardToCaller function uses the .transfer() method to send Ether to the caller. This method is discouraged because it forwards a fixed amount of gas (2300 gas), which may not be sufficient for the recipient to execute necessary logic upon receiving Ether. This can lead to failed transactions and compatibility issues with smart contract wallets.

Root Cause

Using msg.sender.transfer(amount) imposes a fixed gas stipend, which can cause the Ether transfer to fail if the recipient is a contract that requires more gas to process the transaction. This practice is outdated and can lead to Denial-of-Service (DoS) vulnerabilities.

Code Location

• File: MorphoLeverageStrategyExtension.sol
• Function: _transferEtherRewardToCaller

function _transferEtherRewardToCaller(uint256 _etherReward) internal returns(uint256) {
    uint256 etherToTransfer = _etherReward < address(this).balance ? _etherReward : address(this).balance;

    msg.sender.transfer(etherToTransfer);

    return etherToTransfer;
}

Internal Pre-conditions

• The contract has a positive Ether balance.
• _transferEtherRewardToCaller is called to reward the caller.

External Pre-conditions

• The caller is an EOA or a contract that may require more than 2300 gas to process incoming Ether.

Attack Path

1. Invocation: A user or contract triggers a function that leads to _transferEtherRewardToCaller being called.
2. Transfer Failure: If the recipient is a contract that needs more than 2300 gas, the transfer fails.
3. Denial of Service: Legitimate users are unable to receive rewards, leading to failed transactions.

Impact

• Failed Transactions: Users may be unable to receive Ether rewards.
• Compatibility Issues: Smart contract wallets cannot interact properly.
• User Frustration: Users may lose trust in the contract.

Estimated Loss

No direct financial loss to the contract but potential loss of user trust and decreased usability.

Mitigation

• Use .call{value: amount}(""): Replace .transfer() with .call() to send Ether, forwarding all available gas.

  function _transferEtherRewardToCaller(uint256 _etherReward) internal returns(uint256) {
      uint256 etherToTransfer = _etherReward < address(this).balance ? _etherReward : address(this).balance;
  
      (bool success, ) = msg.sender.call{value: etherToTransfer}("");
      require(success, "Ether transfer failed");
  
      return etherToTransfer;
  }

• Handle Failures Gracefully: Ensure the contract handles failed transfers appropriately.

Conclusion

Using .transfer() is outdated and can lead to failed Ether transfers. Switching to .call{value: amount}("") ensures better compatibility and reliability, enhancing the contract's usability and security.