Use of transfer() for ETH Withdrawal May Lead to Stuck Funds

Summary

The contract currently uses Solidity’s transfer() function to handle ETH withdrawals, which may lead to failed transactions under certain conditions, potentially causing user funds to become stuck.

Vulnerability Details

In the TokenManager::withdraw function, the contract utilizes the transfer() function to send ETH to users. However, this approach has limitations, especially when interacting with other smart contracts.

Key Code Section:

Problems with transfer():

• Fixed Gas Stipend: The transfer() function forwards a fixed gas stipend of 2300 gas units. If the recipient contract's fallback or receive function requires more than 2300 gas units to execute, the transfer will fail.

• Lack of Flexibility: Some smart contracts may have custom logic in their fallback or receive functions, requiring more gas than what transfer() provides.

• Potential Stuck Funds: If a smart contract recipient does not implement a payable fallback function or requires more gas, the withdrawal will fail, potentially causing user funds to be stuck in the contract.

Reference:

This issue is well-documented in the Ethereum community, as using transfer() can lead to failed transactions in situations where more gas is required.

Impact

Users may be unable to withdraw their ETH if they are using a smart contract address that does not conform to the conditions required by transfer(). This can result in a loss of access to funds.

Tools Used

Manual code review.

Recommendations

1. Use call() for ETH Transfers: Replace transfer() with call() to provide more flexibility and prevent potential gas issues. The call() function allows specifying the gas limit, ensuring that the transfer can succeed even if the recipient requires more than 2300 gas units.

2. Check for Success: When using call(), ensure that the transfer was successful by checking the returned result:

   (bool success, ) = msg.sender.call{value: claimAbleAmount}("");

   require(success, "ETH transfer failed");

3. Consider OpenZeppelin's Address.sendValue(): As an alternative, use OpenZeppelin's Address.sendValue() function, which is designed to mitigate these issues by using the low-level call() method internally:

   Address.sendValue(payable(msg.sender), claimAbleAmount);

Implementing these recommendations will make the contract more robust and prevent issues related to ETH withdrawals.