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

Users can get NFTs sending zero transactions.

blacksquirrel

Summary

The donate function lacks a check for msg.value, allowing users to obtain an NFT by sending a transaction with zero ether.

Vulnerability Details

Proof of code:

Add this code to tests , but as there is a bug in the constructor of the GivingThankscontract , fix it before running tests .

constructor(address _registry) ERC721("DonationReceipt", "DRC") {
+ registry = CharityRegistry(_registry);
- registry = CharityRegistry(_msg.sender);
owner = msg.sender;
tokenCounter = 0;
}
function testZeroDonate() public {
// Check initial token counter
uint256 initialTokenCounter = charityContract.tokenCounter();
​
// Donor donates to the charity zero Ether
vm.prank(donor);
charityContract.donate{value: 0}(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);
​
// Verify that the zero donation was sent to the charity
uint256 charityBalance = charity.balance;
assertEq(charityBalance, 0);
}

Impact

Without a check for msg.value, users can call the function donate with zero ether and still receive an NFT.

Tools Used

Manual code review.

Recommendations

To address this vulnerability, you should add a check for msg.value in the donate function.

function donate(address charity) public payable {
require(msg.value > 0,"Amount is O");
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.