GivingThanks

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

The user can call the function donate(address charity) and send 0 ether.

Summary

The donate function in the GivingThanks contract does not enforce a minimum Ether amount requirement. This allows users to call the function with msg.value of 0, which means they can receive an NFT without making an actual donation. This vulnerability can lead to abuse of the NFT minting process, potentially allowing users to flood the system with NFTs without contributing any funds to the intended charities.

Vulnerability Details

The donate function currently lacks a check for a minimum msg.value:

function donate(address charity) public payable {
registry.returnRegistry(charity);
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;
}

Without a require statement to check if msg.value > 0, users can call this function with zero Ether and still mint an NFT.

Impact

This issue has significant implications:

  • NFT Abuse: Users can mint NFTs for free by sending zero Ether, potentially flooding the system with worthless NFTs.

  • Financial Loss for Charities: The intended purpose of donating to charities is undermined, as users can exploit the function without actually contributing funds.

  • Reputational Risk: Allowing "zero-donation" NFT mints may reduce trust in the platform's ability to ensure funds are being properly directed to charities.

POC

Input

function testVerify() public {
address unverifiedCharity = address(0x4);
uint256 amount = 0 ether;
vm.deal(donor, 100000 ether);
vm.prank(admin);
registryContract = new CharityRegistry();
vm.prank(admin);
charityContract = new GivingThanks(address(registryContract));
vm.prank(admin);
registryContract.registerCharity(unverifiedCharity);
vm.prank(donor);
for(uint256 i = 0; i<=5; i++){
charityContract.donate{value:amount}(unverifiedCharity);
}
}

Output

Ran 1 test for test/GivingThanks.t.sol:GivingThanksTest
[PASS] testVerify() (gas: 2940886)
Traces:
[2940886] GivingThanksTest::testVerify()
├─ [0] VM::deal(donor: [0xBd4D11D884c3c7bF373b013AC52fd99f9DD86D0A], 100000000000000000000000 [1e23])
│ └─ ← [Return]
├─ [0] VM::prank(admin: [0xaA10a84CE7d9AE517a52c6d5cA153b369Af99ecF])
│ └─ ← [Return]
├─ [204961] → new CharityRegistry@0x3Ede3eCa2a72B3aeCC820E955B36f38437D01395
│ └─ ← [Return] 913 bytes of code
├─ [0] VM::prank(admin: [0xaA10a84CE7d9AE517a52c6d5cA153b369Af99ecF])
│ └─ ← [Return]
├─ [1452020] → new GivingThanks@0x6D9da78B6A5BEdcA287AA5d49613bA36b90c15C4
│ └─ ← [Return] 6794 bytes of code
├─ [0] VM::prank(admin: [0xaA10a84CE7d9AE517a52c6d5cA153b369Af99ecF])
│ └─ ← [Return]
├─ [22508] CharityRegistry::registerCharity(Identity: [0x0000000000000000000000000000000000000004])
│ └─ ← [Stop]
├─ [0] VM::prank(donor: [0xBd4D11D884c3c7bF373b013AC52fd99f9DD86D0A])
│ └─ ← [Return]
├─ [223664] GivingThanks::donate(Identity: [0x0000000000000000000000000000000000000004])
│ ├─ [569] CharityRegistry::isVerified(Identity: [0x0000000000000000000000000000000000000004]) [staticcall]
│ │ └─ ← [Return] true
│ ├─ [15] PRECOMPILES::identity(0x)
│ │ └─ ← [Return]
│ ├─ emit Transfer(from: 0x0000000000000000000000000000000000000000, to: donor: [0xBd4D11D884c3c7bF373b013AC52fd99f9DD86D0A], tokenId: 0)
│ ├─ emit MetadataUpdate(_tokenId: 0)
│ └─ ← [Stop]
├─ [203764] GivingThanks::donate(Identity: [0x0000000000000000000000000000000000000004])
│ ├─ [569] CharityRegistry::isVerified(Identity: [0x0000000000000000000000000000000000000004]) [staticcall]
│ │ └─ ← [Return] true
│ ├─ [15] PRECOMPILES::identity(0x)
│ │ └─ ← [Return]
│ ├─ emit Transfer(from: 0x0000000000000000000000000000000000000000, to: GivingThanksTest: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496], tokenId: 1)
│ ├─ emit MetadataUpdate(_tokenId: 1)
│ └─ ← [Stop]
├─ [181864] GivingThanks::donate(Identity: [0x0000000000000000000000000000000000000004])
│ ├─ [569] CharityRegistry::isVerified(Identity: [0x0000000000000000000000000000000000000004]) [staticcall]
│ │ └─ ← [Return] true
│ ├─ [15] PRECOMPILES::identity(0x)
│ │ └─ ← [Return]
│ ├─ emit Transfer(from: 0x0000000000000000000000000000000000000000, to: GivingThanksTest: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496], tokenId: 2)
│ ├─ emit MetadataUpdate(_tokenId: 2)
│ └─ ← [Stop]
├─ [181864] GivingThanks::donate(Identity: [0x0000000000000000000000000000000000000004])
│ ├─ [569] CharityRegistry::isVerified(Identity: [0x0000000000000000000000000000000000000004]) [staticcall]
│ │ └─ ← [Return] true
│ ├─ [15] PRECOMPILES::identity(0x)
│ │ └─ ← [Return]
│ ├─ emit Transfer(from: 0x0000000000000000000000000000000000000000, to: GivingThanksTest: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496], tokenId: 3)
│ ├─ emit MetadataUpdate(_tokenId: 3)
│ └─ ← [Stop]
├─ [181864] GivingThanks::donate(Identity: [0x0000000000000000000000000000000000000004])
│ ├─ [569] CharityRegistry::isVerified(Identity: [0x0000000000000000000000000000000000000004]) [staticcall]
│ │ └─ ← [Return] true
│ ├─ [15] PRECOMPILES::identity(0x)
│ │ └─ ← [Return]
│ ├─ emit Transfer(from: 0x0000000000000000000000000000000000000000, to: GivingThanksTest: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496], tokenId: 4)
│ ├─ emit MetadataUpdate(_tokenId: 4)
│ └─ ← [Stop]
├─ [181864] GivingThanks::donate(Identity: [0x0000000000000000000000000000000000000004])
│ ├─ [569] CharityRegistry::isVerified(Identity: [0x0000000000000000000000000000000000000004]) [staticcall]
│ │ └─ ← [Return] true
│ ├─ [15] PRECOMPILES::identity(0x)
│ │ └─ ← [Return]
│ ├─ emit Transfer(from: 0x0000000000000000000000000000000000000000, to: GivingThanksTest: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496], tokenId: 5)
│ ├─ emit MetadataUpdate(_tokenId: 5)
│ └─ ← [Stop]
└─ ← [Stop]
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.80ms (1.21ms CPU time)
Ran 1 test suite in 597.67ms (1.80ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

Tools Used

Manual

Recommendations

Add a Minimum Donation Check: Ensure msg.value > 0 to require a positive Ether amount for NFT minting.

require(msg.value > 0, "Donation amount must be greater than zero");
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.