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

Wrong implementation of verification function opens up the way for donators to send money to unverified addresses unknowingly

0xgondar

Summary

CharityRegistry::isVerified view function returns registeredCharities, instead of verifiedCharities. An attacker can trick users to send donations to their unverified charity as anyone can register a charity, but only admin can verify them.

Vulnerability Details

CharityRegistry::isVerified returns the wrong mapping:

function isVerified(address charity) public view returns (bool) {
return registeredCharities[charity];
}
​

While at first this seems like a trivial issue, if you look at GivingThanks::donate function and consider a specific scenario, it becomes quite serious.

GivingThanks::donate function checks if a charity is verified or not.

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");
​
_mint(msg.sender, tokenCounter);
​
// Create metadata for the tokenURI
string memory uri = _createTokenURI(msg.sender, block.timestamp, msg.value);
_setTokenURI(tokenCounter, uri);
​
tokenCounter += 1;
}

Consider the following scenario: Alice, a donator, trusts the platform's admin and verification status. She decides to make a donation to a charity.
Bob, a malicious actor, registers a bogus charity and somehow persuades Alice that it is a geniune charity. Mind that Alice, even if she does not trust Bob, trusts the protocol and sends donation to that bogus address. Since the require(registry.isVerified(charity), "Charity not verified"); line is present, she believes she's in the safe spot.

But since CharityRegistry::isVerified returns true in registered addresses, that can be registered by anyone and anytime, and NOT the verified, trusted addresses, Alice will be tricked into sending her money into a bogus charity address.

Impact

Users will get their money stolen, and the protocol's credibility will suffer immensely.

Tools Used

Manual review.

Recommendations

Fix the CharityRegistry::isVerified function as follows:

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

Lead Judging Commences

n0kto Lead Judge 12 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.