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

[H-02] `CharityRegistry::isVerified` incorrectly returns the result from the `Charity::registeredCharities` mapping instead of the `CharityRegistry::verifiedCharities` mapping

0xnelli

Summary

The function CharityRegistry::isVerified does not return whether the charity is verified, instead it returns whether the charity has been registered, as it returns the value from the incorrect mapping.

Vulnerability Details

The function CharityRegistry::isVerified returns the Boolean result of the CharityRegistry::registeredCharities mapping.

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

This is not the intended purpose of the function, as anyone can register an address as charity:

function registerCharity(address charity) public {
registeredCharities[charity] = true;
}

The intention of the CharityRegistry::isVerified function is return registered charity addresses that have been verified by the admin, which are stored in CharityRegistry::verifiedCharities.

function verifyCharity(address charity) public {
require(msg.sender == admin, "Only admin can verify");
require(registeredCharities[charity], "Charity not registered");
@> verifiedCharities[charity] = true;
}

This prevents donors from donating to fraudulent addresses posing as a charity, as can be seen by the check made at the beginning of GivingThanks::donate below:

function donate(address charity) public payable {
@> require(registry.isVerified(charity), "Charity not verified");
​
...
​
}

However, due to the wrong mapping value being returned, donors can donate to registered charities that have not been verified.

Impact

Donors can donate to unverified charities in the GivingThanks::donate function, as a charity only needs to be registered to pass the function's checks.

Proof of Code

Run the following test:

function testCannotDonateToUnverifiedCharity() public {
address unverifiedCharity = address(0x4);
​
// Unverified charity registers but is not verified
vm.prank(unverifiedCharity);
registryContract.registerCharity(unverifiedCharity);
​
vm.deal(donor, 10 ether);
​
// Donor tries to donate to unverified charity
vm.prank(donor);
vm.expectRevert();
charityContract.donate{value: 1 ether}(unverifiedCharity);
}

Output:

Ran 1 test for test/GivingThanks.t.sol:GivingThanksTest
[FAIL. Reason: call did not revert as expected] testCannotDonateToUnverifiedCharity() (gas: 307606)
Traces:
[307606] GivingThanksTest::testCannotDonateToUnverifiedCharity()
├─ [0] VM::prank(0x0000000000000000000000000000000000000004)
│ └─ ← [Return]
├─ [22464] CharityRegistry::registerCharity(0x0000000000000000000000000000000000000004)
│ └─ ← [Stop]
├─ [0] VM::deal(donor: [0xBd4D11D884c3c7bF373b013AC52fd99f9DD86D0A], 10000000000000000000 [1e19])
│ └─ ← [Return]
├─ [0] VM::prank(donor: [0xBd4D11D884c3c7bF373b013AC52fd99f9DD86D0A])
│ └─ ← [Return]
├─ [0] VM::expectRevert(custom error f4844814:)
│ └─ ← [Return]
├─ [262041] GivingThanks::donate{value: 1000000000000000000}(0x0000000000000000000000000000000000000004)
│ ├─ [547] CharityRegistry::isVerified(0x0000000000000000000000000000000000000004) [staticcall]
│ │ └─ ← [Return] true
│ ├─ [15] PRECOMPILES::identity{value: 1000000000000000000}(0x)
│ │ └─ ← [Return]
│ ├─ emit Transfer(from: 0x0000000000000000000000000000000000000000, to: donor: [0xBd4D11D884c3c7bF373b013AC52fd99f9DD86D0A], tokenId: 0)
│ ├─ emit MetadataUpdate(_tokenId: 0)
│ └─ ← [Stop]
└─ ← [Revert] call did not revert as expected

As seen above the donation transaction to a registered but unverified charity did not revert.

Tool Used

Manual review & foundry unit tests

Recommended Mitigation

The CharityRegistry::isVerified function return the value from the CharityRegistry::verifiedRegistries mapping.

function isVerified(address charity) public view returns (bool) {
+ return verifiedCharities[charity];
- return registeredCharities[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.