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

Donate function in GivingThanks does not follow the CEI pattern and does not emit events

ssalaudeen10

Summary

The donate function does not follow the Checks-Effects-Interactions (CEI) pattern. All external interactions, such as using the call function to send ether, should be done after mutating the blockchain state. Additionally, emitting events would be a good addition to help the front end.

Vulnerability Details

bug below

function donate(address charity) public payable {
require(registry.isVerified(charity), "Charity not verified");
require(msg.value > 0, "Zero value is not allowed for donation"); // This should be added
(bool sent, ) = charity.call{value: msg.value}("");
require(sent, "Failed to send Ether");
​
_mint(msg.sender, tokenCounter);
​
// Create metadata for the tokenURI
string memory uri = _createTokenURI(
msg.sender,
block.timestamp,
msg.value
);
_setTokenURI(tokenCounter, uri);
​
tokenCounter += 1;
}

Solution
Following the CEI pattern to prevent reentrancy attacks and emitting events for better frontend interaction

function donate(address charity) public payable {
​
//Checks
require(registry.isVerified(charity), "Charity not verified");
require(msg.value > 0, "Zero value is not allowed for donation"); // This should be added
// Effects
_mint(msg.sender, tokenCounter);
​
// Create metadata for the tokenURI
string memory uri = _createTokenURI(
msg.sender,
block.timestamp,
msg.value
);
_setTokenURI(tokenCounter, uri);
​
tokenCounter += 1;
// Interactions
​
(bool sent, ) = charity.call{value: msg.value}("");
require(sent, "Failed to send Ether");
//Emit event
emit Donated(msg.sender, charity, msg.value);
}

Impact

Not following the CEI pattern can lead to reentrancy attacks, which could compromise the security of the contract. Ensuring all interactions are done last minimizes this risk.

Tools Used

Foundry Test

Recommendations

  • Follow the CEI pattern to ensure external interactions are handled safely.

  • Emit event logs when the blockchain state changes to aid frontend development and user interaction tracking.

Updates

Lead Judging Commences

n0kto Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Too generic

Support

FAQs

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