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

Bug Report: Broken Verification Logic in isVerified Function of CharityRegistry.sol

koinerdegen

Summary:
The isVerified function in CharityRegistry.sol mistakenly checks the registeredCharities mapping instead of the verifiedCharities mapping. As a result, the GivingThanks contract allows donations to any registered charity regardless of whether it has been verified, bypassing the intended verification requirement.

Vulnerability Details:
The function isVerified in CharityRegistry.sol is designed to validate if a charity is verified before allowing a donation. However, instead of checking the verifiedCharities mapping, which stores verification status, it incorrectly references registeredCharities, a separate mapping for registration status. This oversight breaks the workflow, permitting donations to any registered charity, verified or not.

Vulnerable Code

The error is found in the isVerified function within CharityRegistry.sol:

function isVerified(address charity) public view returns (bool) {
return registeredCharities[charity]; // Incorrect mapping used here
}
​

This should reference verifiedCharities instead:

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

Impact:

This vulnerability allows donations to any charity that is merely registered without being verified. This opens the contract to potential misuse or fraudulent donations to unverified charities, undermining the integrity of the donation process and the intent of the verification system. Given the implications for donor trust and potential misuse of funds, this bug can be categorized as a HIGH-LEVEL SEVERITY ISSUE.

Tools Used:

  • Manual code inspection

  • forge test output and trace analysis

Recommendations:
To resolve this issue, update the isVerified function to check the correct mapping (verifiedCharities). This fix aligns the logic with the intended functionality, ensuring donations are only permitted for verified charities.

Solution

In CharityRegistry.sol, modify the isVerified function as follows:

// Corrected function to reference verifiedCharities
function isVerified(address charity) public view returns (bool) {
return verifiedCharities[charity];
}

This change enforces the intended verification check, preventing donations to unverified charities and restoring the expected workflow of the contract.

Notice to Command:

During testing, the function testCannotDonateToUnverifiedCharity failed as it allowed donations to an unverified charity, indicating that verification was not enforced as intended. Below is the command run and the detailed output:

Command and Test Output:

  1. Initial command run:

forge test

Output:

Suite result: FAILED. 2 passed; 1 failed; 0 skipped; finished in 30.75ms (27.43ms CPU time)
[FAIL: next call did not revert as expected] testCannotDonateToUnverifiedCharity() (gas: 307608)
​

to gain more detail, the following was run:

forge test --mt testCannotDonateToUnverifiedCharity -vvvv
​

Output:

[307608] GivingThanksTest::testCannotDonateToUnverifiedCharity()
├─ [0] VM::prank(Identity: [0x0000000000000000000000000000000000000004])
├─ [22464] CharityRegistry::registerCharity(Identity: [0x0000000000000000000000000000000000000004])
├─ [0] VM::deal(donor: [0xBd4D11D884c3c7bF373b013AC52fd99f9DD86D0A], 10000000000000000000 [1e19])
├─ [0] VM::prank(donor: [0xBd4D11D884c3c7bF373b013AC52fd99f9DD86D0A])
├─ [0] VM::expectRevert(custom error 0xf4844814)
├─ [262040] GivingThanks::donate{value: 1000000000000000000}(Identity: [0x0000000000000000000000000000000000000004])
│ ├─ CharityRegistry::isVerified(...) // incorrectly returns true
│ └─ ← [Stop]
​

From this output, it's clear that CharityRegistry::isVerified returned true for an unverified charity, allowing the donation. This confirmed that the test failed due to the incorrect mapping reference.

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.