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

Lack of check on `msg.value` within `GivingThanks::donate`

somasine

Summary

GivingThanks::donate is intended to allow donors to contribute funds to a verified charity. However there isn't any check for whether that donor is actually sending any funds.

Impact

Likelihood: Low
Severity: Low

The likelihood that a user executes donate without any funds is very low and would normally be pointlesss. However it should be worth mentioning that if there are any future plans on benefits for user that have a donation NFT, then adding a requirement for users to actually donate funds might be important.

Tools Used

  • Manual Review

Recommendations

Please add the following require() statement to the following GivingThanks::donate function:

function donate(address charity) public payable {
+ require(msg.value > 0, "Donation must greater than 0");
require(registry.isVerified(charity), "Charity not verified");
(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;
}
Updates

Lead Judging Commences

n0kto Lead Judge 10 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-0-donation-mint-an-NFT

Likelyhood: Low, anyone can mint an NFT with 0 amount. No reason to do it. Impact: Informational/Very Low, NFT are minted to a false donator. An NFT with 0 in the amount section would be useless. Since that's a bad design and not expected, I'll consider it Low but in a real contest, it could be informational because there is no real impact.

Support

FAQs

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