0xSolus - Use call instead of transfer for withdrawals.

[sherlock-admin2]

0xSolus

Medium

Use call instead of transfer for withdrawals.

Summary

EthosReview::withdrawFunds() currently uses transfer to send Ether to the onlyOwner. The transfer function imposes a fixed gas limit of 2300 gas for the transfer, which can lead to transaction failures if the recipient is a contract and executes some custom logic on fallback() or receive().

Root Cause

No response

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

No response

Impact

Gnosis Safe multisig wallets, as well as other smart contract wallets, generally require more than 2300 gas due to their internal operations.

This issue could lead to failed withdrawals by the contract owner due to the gas limit restriction. If the transfer fails, the contract owner will be unable to retrieve Ether stored in the contract.

Note: I'm adding this as medium as team, declared that they are planning to use a multisig for onlyOwner.

PoC

No response

Mitigation

To ensure successful withdrawals and avoid issues related to the fixed gas limit, replace transfer with call, which allows specifying a custom gas limit. This approach is more flexible and compatible with smart contract wallets like Gnosis Safe.

  /**
   * @dev Withdraws funds.
   * @param paymentToken Payment token address.
   */
  function withdrawFunds(address paymentToken) external onlyOwner {
    if (paymentToken == address(0)) {
        (bool success, ) = msg.sender.call{value: address(this).balance}("");
        require(success, "Ether transfer failed");
    } else {
      IERC20(paymentToken).transfer(msg.sender, IERC20(paymentToken).balanceOf(address(this)));
    }
  }