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

Wrong return function in `CharityRegistry:isVerified` output incorrect and misleading verification status of a charity

soupy

Summary

The return function in CharityRegistry:isVerified was found incorrectly implemented. Instead of returning the verification status of a charity, it returns the registration status of a charity. This leads to misleading information to any user who calls this function to check if a charity is legitimate and if to donate to it.

https://github.com/Cyfrin/2024-11-giving-thanks/blob/304812abfc16df934249ecd4cd8dea38568a625d/src/CharityRegistry.sol#L23-L25

Vulnerability Details

CharityRegistry:isVerified has wrongly implemented its return function. Instead of returning the verification status, it returns the registration status of a charity

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

Proof of Concept:

Add the following test to test\GivingThanks.t.sol:

function test_audit_returnFunctionInIsVerifiedCharityRegistry() public {
address mockCharity = makeAddr("mockCharity");
address randomUser = makeAddr("randomUser");
​
// a random user register a new charity
vm.prank(randomUser);
registryContract.registerCharity(mockCharity);
​
// a donor gets interested in the charity and check the verification status of the new charity
vm.prank(donor);
bool verifyRegistry = registryContract.isVerified(mockCharity);
​
// verify outcome at this stage is expected to return false since admin has not done verification
assertEq(verifyRegistry, false);
}

Run the test forge test --match-test test_audit_returnFunctionInIsVerifiedCharityRegistry

Ran 1 test for test/GivingThanks.t.sol:GivingThanksTest
[FAIL: assertion failed: true != false] test_audit_returnFunctionInIsVerifiedCharityRegistry() (gas: 37870)
Suite result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 1.43ms (109.96µs CPU time)

The test failed indicating that the verification status didn't reflect the expected status which it should return false as the charity had not been verified by the admin after its registration.

Impact

Wrong return verification status that misleads any users/donors who intend to donate to the charity thinking the charity organization eligibility/validity has been verified, damaging the credibility of the protocol to carry out its due diligence.

Tools Used

Manual review with test

Recommendations

Amend the return function to return the correct verification status

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

Rerun the test above forge test --match-test test_audit_returnFunctionInIsVerifiedCharityRegistry

Ran 1 test for test/GivingThanks.t.sol:GivingThanksTest
[PASS] test_audit_returnFunctionInIsVerifiedCharityRegistry() (gas: 39879)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 571.00µs (105.21µs CPU time)

The test passed indicating that the change recommended does rectify the error and reflect the correct verification status.

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.