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

Users can donate to unverified charities and get NFTs for it.

blacksquirrel

Summary

A critical issue has been identified in the implementation of the donate function, stemming from a flaw in the isVerified function of the CharityRegistry contract.

Vulnerability Details

The donate function contains a verification check for the recipient address. However, the isVerified function in the CharityRegistry contract is incorrectly implemented. As a result, donations can be successfully processed for both registered and unregistered addresses. This vulnerability leads to unauthorized NFT creation and donation for unverified recipients.

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 testCanDepositToUnverified() public {
// Check initial token counter
uint256 initialTokenCounter = charityContract.tokenCounter();
uint256 donationAmount = 1 ether;
​
address unverifiedCharity = address(0x4);
​
// Unverified charity registed but is not verified
vm.prank(donor);
registryContract.registerCharity(unverifiedCharity);
​
// Fund the donor
vm.deal(donor, 10 ether);
​
// Donor tries to donate to unverified charity
vm.prank(donor);
​
charityContract.donate{value: 1 ether}(unverifiedCharity);
​
// 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 donation was sent to the charity
uint256 charityBalance = unverifiedCharity.balance;
assertEq(charityBalance, donationAmount);
}
}
```

Impact

Due to this error, it becomes possible for donations to be made to unverified addresses, resulting in the improper issuance of NFTs and donations .

Tools Used

Manual code review

Recommendations

Review and correct the implementation of the isVerified function in the CharityRegistry contract.

function isVerified(address charity) public view returns (bool) {
+ return verifiedCharities[charity];
- return registeredCharities[charity];
}
Updates

Lead Judging Commences

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

finding-isVerified-return-registered-charities

Likelyhood: High, the function returns registered charities instead of verified ones. Impact: High, Any charities can be registered by anyone and will be declared as verified by this function bypassing verification.

Support

FAQs

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