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

The function donate not fallows the CEI pattern.

eth0x

Summary

The donate function serves a crucial role in facilitating the donation process while providing donors with a tangible reward for their contribution.

Vulnerability Details

In the current donate function, the interaction with the charity (sending Ether) happens before the state is updated:

solidity

function donate(address charity) public payable {
require(registry.isVerified(charity), "Charity not verified");
// Interaction
(bool sent,) = charity.call{value: msg.value}("");
require(sent, "Failed to send Ether");
​
// Effects (state changes)
_mint(msg.sender, tokenCounter);
string memory uri = _createTokenURI(msg.sender, block.timestamp, msg.value);
_setTokenURI(tokenCounter, uri);
tokenCounter += 1;
}

Impact

Reentrancy Attacks:

Problem: When a smart contract makes an external call (e.g., sending Ether) before completing state changes, the receiving contract can call back the original contract before it finishes its execution.

Consequence: An attacker can repeatedly invoke the function, leading to unintended or unauthorized operations, such as multiple Ether withdrawals.

Inconsistent State:

Problem: Updating the state after performing an external interaction can lead to state inconsistencies if the external call fails.

Consequence: The contract's state may not reflect the actual transaction status, potentially causing logical errors.

Security and Trust:

Problem: Failing to follow the CEI pattern can expose the contract to various security vulnerabilities, potentially undermining user trust in the system.

Consequence: Loss of user trust and potential financial losses.

Tools Used

manual review

Recommendations

Here’s how you can refactor the function to follow the CEI pattern:

function donate(address charity) public payable {
// Checks
require(registry.isVerified(charity), "Charity not verified");
require(msg.value > 0, "Donation must be greater than 0");
​
// Effects (state changes)
_mint(msg.sender, tokenCounter);
string memory uri = _createTokenURI(msg.sender, block.timestamp, msg.value);
_setTokenURI(tokenCounter, uri);
tokenCounter += 1;
​
// Interaction
(bool sent,) = charity.call{value: msg.value}("");
require(sent, "Failed to send Ether");
}
Updates

Lead Judging Commences

n0kto Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Too generic

Support

FAQs

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