GivingThanks
First Flight #28
Beginner FriendlyFoundry
100 EXP
View results
Previous Next
Submission Details
Severity: medium
Invalid

[M-2] Potential Silent Failure on Donation Transfer in `GivingThanks::donate`

affanimran1303

Description:

The GivingThanks::donate Ether transfer to a charity might fail without clear feedback.

Impact:

The donate function performs a low-level call to send Ether to the charity but does not handle failures effectively. If the transfer fails, the contract currently does not log or revert in a way that informs users or donors. While the code includes a require(sent, "Failed to send Ether") check, adding an event to log failures would improve transparency.

Proof Of Code:

function test_DonationTransferFailure()public{
GivingThanks givingThanks = new GivingThanks(address(this));
address charity = address(0);
vm.expectRevert("Failed to send ETHER");
givingThanks.donate{value:1 ether}(charity);
}

This test confirms that a failure in the transfer to the charity address causes a revert with an appropriate error message, helping prevent silent failures in donation transfers.

Tools Used:

Foundry

Recommended Mitigation:

  1. Log Success and Failure Events:
    Create two events, one for successful transfers and one for failures, to notify donors of the donation outcome.
    Emit a DonationSuccess event when the transfer is successful, and a DonationFailure event if the transfer fails.

  2. Fallback Mechanism or Refund (Optional):
    If a transfer fails, an optional fallback mechanism (like adding funds to a refund balance) could be implemented, allowing users to withdraw their donation if the initial transfer to the charity fails.

// Define the new events
event DonationSuccess(address indexed donor, address indexed charity, uint256 amount);
event DonationFailure(address indexed donor, address indexed charity, uint256 amount, string reason);
​
function donate(address charity) public payable {
require(registry.isVerified(charity), "Charity not verified");
​
// Attempt to transfer the donation
(bool sent, ) = charity.call{value: msg.value}("");
if (sent) {
emit DonationSuccess(msg.sender, charity, msg.value);
// Checks-Effects-Interactions Pattern
uint256 tokenId = tokenCounter;
tokenCounter += 1; // Effects: Update state before external interactions
// Mint a receipt NFT upon successful donation
_mint(msg.sender, tokenCounter);
// Create metadata for the tokenURI
string memory uri = _createTokenURI(msg.sender, block.timestamp, msg.value);
_setTokenURI(tokenCounter, uri);
​
} else {
emit DonationFailure(msg.sender, charity, msg.value, "Failed to send Ether");
revert("Failed to send Ether");
}
}

By logging both successful and failed attempts, donors are clearly informed of the status of their transactions, reducing the likelihood of confusion. This improves user experience and adds a layer of accountability and transparency.

Updates

Lead Judging Commences

n0kto Lead Judge 8 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.