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

The function `GivingThanks::donate` doesn't validate that `msg.value` must be greater than zero.

xilobytes

Description The GivingThanks::donate function never checks that the donation amount(msg.value) is greater than zero.

Impact This issue leads to users being able of mint "DonatioReceipt" tokens without actually donating any ETH to the charity.

Proof of Concepts

Add the following test to Test.t.sol.

function testCanDonateZeroETHAndStillReceiveNFT() public {
uint256 donationAmount = 0 ether;
​
// Check initial token counter
uint256 initialTokenCounter = charityContract.tokenCounter();
assertEq(initialTokenCounter, 0);
​
// Donor donates to the charity
vm.prank(donor);
charityContract.donate{value: donationAmount}(charity);
​
// Check that the NFT was minted
uint256 newTokenCounter = charityContract.tokenCounter();
assertEq(newTokenCounter, initialTokenCounter + 1);
​
// Verify ownership of the NFT
address ownerOfToken = charityContract.ownerOf(initialTokenCounter);
assertEq(ownerOfToken, donor);
}

Recommended mitigation Add a proper validation for msg.value.

function donate(address charity) public payable {
+ require(msg.value > 0, "Needs to send some ETH");
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 8 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.