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

Zero-Ether Donations Allow Free NFT Minting

delinked

Summary

The donate function in GivingThanks.sol allows a zero-Ether donation, which mints an NFT for the donor without requiring any actual funds to be contributed. This could enable abuse by allowing malicious actors to mint unlimited NFTs at no cost, undermining the donation platform’s integrity.

Vulnerability Details

The donate function currently lacks a check to ensure that msg.value (the donated amount) is greater than zero. Without this check, a user can call donate with zero Ether, allowing them to mint NFTs without making any actual donation. This issue opens the platform to exploitation, allowing unauthorized and unlimited NFT minting.

Here is a proof of code that shows a test case in which a donor makes a contribution with a value of Zero, but is still minted the NFT correctly, also correctly updating the tokenCounter, and giving ownership of that NFT to the donor.

function testDonateZeroAmount() public {
uint256 initialTokenCounter = charityContract.tokenCounter();
​
// Fund the donor with Ether (though we won't send any in this test)
vm.deal(donor, 10 ether);
​
// Log initial values
console.log("Initial tokenCounter:", initialTokenCounter);
console.log("Donor address:", donor);
​
// Donor attempts to donate 0 Ether to the charity
vm.prank(donor);
charityContract.donate{value: 0}(charity);
​
// Check that the tokenCounter has incremented, indicating an NFT was minted
uint256 newTokenCounter = charityContract.tokenCounter();
console.log("New tokenCounter:", newTokenCounter);
​
// Verify ownership of the NFT
address ownerOfToken = charityContract.ownerOf(initialTokenCounter);
console.log("Owner of NFT token:", ownerOfToken);
console.log("Expected donor address:", donor);
​
assertEq(newTokenCounter, initialTokenCounter + 1);
assertEq(ownerOfToken, donor);
​
// Check that the charity's balance remains unchanged
uint256 charityBalance = charity.balance;
console.log("Charity balance:", charityBalance);
assertEq(charityBalance, 0);
}

Here are the result and the logs from that test:

[PASS] testDonateZeroAmount() (gas: 259626)
Logs:
Initial tokenCounter: 0
Donor address: 0xBd4D11D884c3c7bF373b013AC52fd99f9DD86D0A
New tokenCounter: 1
Owner of NFT token: 0xBd4D11D884c3c7bF373b013AC52fd99f9DD86D0A
Expected donor address: 0xBd4D11D884c3c7bF373b013AC52fd99f9DD86D0A
Charity balance: 0

Impact

By allowing zero-Ether donations, the platform is open to abuse, where attackers can mint unlimited NFTs at no cost. This can flood the platform with free NFTs, diminish the perceived value of genuine donations, and undermine the trust and integrity of the system.

Tools Used

  • Manual code review.

  • Test case testDonateZeroAmount to confirm vulnerability using Foundry.

Recommendations

To prevent this vulnerability, add a check in the donate function to ensure that msg.value is greater than zero:

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

This simple check will prevent the function from minting NFTs without an actual donation, safeguarding the platform’s integrity. And also, the test from earlier also fails accordingly.

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.