GivingThanks

First Flight #28
Beginner FriendlyFoundry
100 EXP
View results
Submission Details
Severity: high
Invalid

Unsafe Low-Level Call in donate() Function in GivingThanks.sol

Summary

The donate() function in GivingThanks.sol employs an unsafe low-level .call to transfer funds to a specified charity address without validating that the address is associated with a deployed contract. According to the Solidity documentation, low-level calls do not revert if the target address is non-existent; they instead return true, resulting in a misleading success message even when funds are sent to an invalid or unintended address. This flaw exposes the contract to critical financial risk, as it could lead to permanent loss of donations if the charity address is not properly validated, undermining the platform’s security and credibility.

Vulnerability Details

(bool sent, ) = charity.call{value: msg.value}("");
require(sent, "Failed to send Ether");

In this code, the donate() function performs a low-level .call to send ether to the charity address. No checks are implemented to ensure that the charity address is valid or associated with a deployed contract. Because low-level calls bypass standard validation, they may appear to succeed even if the charity address is a non-existent contract, resulting in funds being sent to an unintended or invalid address without a failure notice. This issue is particularly dangerous in a donation platform, where unverified addresses could cause donations to be lost permanently.

Impact

Loss of Donor Funds: If an invalid or non-existent address is supplied, donated funds will be misdirected or lost without any possibility of recovery.

  • Platform Integrity Risk: Donors expect secure and reliable transactions. This flaw undermines confidence in the platform, as users may not realize their donations are failing silently due to inadequate address validation.

  • Critical Financial Impact: Loss of funds to invalid addresses impacts both the charity’s resources and the platform’s operational integrity, leading to possible legal implications if funds cannot be traced.

Tools Used

** . Solidity Documentation**: Examined to understand the behavior of low-level calls in relation to non-existent contract addresses.

  • Manual Code Review: Identified the potential for silent failure due to lack of address validation.

Recommendations

To prevent donation loss, implement robust address validation by verifying that the charity address has deployed code before transferring funds. Use Solidity’s Address.isContract utility from the OpenZeppelin library or an equivalent solution to confirm that charity is a contract address, not a non-existent or unintended address.

  1. Address Validation:

    • Include a check to ensure charity is an existing contract. The following example utilizes Address.isContract:

      require(Address.isContract(charity), "Invalid charity address");
  2. Safe Transfer Mechanism:

    • Replace the low-level .call with Solidity’s safer transfer or send functions where possible. If .call is necessary, confirm both the validity of the address and the success of the transfer operation.

  3. OpenZeppelin’s Address Utility:

    • Import OpenZeppelin’s Address library to perform contract validation:

      import "@openzeppelin/contracts/utils/Address.sol";
      function donate() public payable {
      require(Address.isContract(charity), "Invalid charity address");
      (bool sent, ) = charity.call{value: msg.value}("");
      require(sent, "Failed to send Ether");
      }
  4. Error Reporting and Logging:

    • Implement an event to log any unsuccessful donations, providing detailed tracking and transparency in case of an issue:

      event DonationFailed(address indexed charity, uint256 amount);
      function donate() public payable {
      require(Address.isContract(charity), "Invalid charity address");
      (bool sent, ) = charity.call{value: msg.value}("");
      if (!sent) {
      emit DonationFailed(charity, msg.value);
      }
      require(sent, "Failed to send Ether");
      }

By implementing these protections, the platform can prevent misdirected donations, secure user funds, and enhance overall trust. This fix addresses both the financial integrity of the platform and safeguards the user experience, ensuring that donations reach the intended verified charities.

Updates

Lead Judging Commences

n0kto Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.