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

Incorrect verification check in the contract CharityRegistry.sol

architect

Summary

The GivingThanks contract contains a vulnerability in the donate function, where it incorrectly verifies the charity address by calling registry.isVerified(charity). This function only checks if the charity is registered, but does not confirm whether the charity has been verified by the admin, as it does not reference the verifiedCharities mapping in CharityRegistry.

Vulnerability Details

In the donate function, the line:

require(registry.isVerified(charity), "Charity not verified");

calls registry.isVerified, which only returns whether a charity is registered:

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

This implementation checks registeredCharities, meaning any registered charity can receive donations, regardless of verification. This error bypasses the intended verification process, where only charities marked as verified by the admin should receive donations.

Impact

The vulnerability allows unverified charities to receive donations, undermining the security and integrity of the contract. Without proper verification, unauthorized or potentially malicious charities can register, bypass verification, and collect funds. This could lead to:

  • Donations being sent to potentially untrustworthy entities: Users expect donations to go only to verified charities, but due to this vulnerability, unverified charities can bypass the intended verification.

  • Loss of trust in the platform: Donors may lose confidence in the system if they discover funds are directed to entities that haven’t undergone a proper vetting process.

Tools Used

Manual

Recommendations

Update the Verification Logic: Modify isVerified in CharityRegistry to confirm both registration and verification statuses. The revised function should check verifiedCharities rather than registeredCharities:

function isVerified(address charity) public view returns (bool) {
return verifiedCharities[charity]; // Ensure charity has been verified, not just registered
}
​

POC

Input

function testVerify() public {
address unverifiedCharity = address(0x4);
uint256 amount = 1 ether;
vm.deal(donor, 10 ether);
​
vm.prank(admin);
registryContract = new CharityRegistry();
vm.prank(admin);
charityContract = new GivingThanks(address(registryContract));
vm.prank(admin);
registryContract.registerCharity(unverifiedCharity);
vm.prank(donor);
​
charityContract.donate{value:amount}(unverifiedCharity);
}
​

Output:

[PASS] testConstructorGivingThanks() (gas: 2047580)
Traces:
[2047580] GivingThanksTest::testConstructorGivingThanks()
├─ [0] VM::deal(donor: [0xBd4D11D884c3c7bF373b013AC52fd99f9DD86D0A], 10000000000000000000 [1e19])
│ └─ ← [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]
├─ [258063] GivingThanks::donate{value: 1000000000000000000}(Identity: [0x0000000000000000000000000000000000000004])
│ ├─ [569] CharityRegistry::isVerified(Identity: [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]
└─ ← [Stop]
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.