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

CharityRegistry::isVerified function returns registered charity status not verified charity, therefore verification is bypassed

bowtiedharpyeagle

Summary

The function isVerified() in the CharityRegistry.sol file returns the registery status of a charity instead of the verified status of the charity. Therefore a potentially scammy charity can still pass as verified without actually being verified and collect funds that never reach the intended charitable causes.

Vulnerability Details

Function isVerified code returns the bool value of the mapping registeredCharities at address charity as seen in the code below:

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

When we check how charities are registered, anyone can register their charity.

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

Therefore this function does not return the status of being verified or not, only the status of being registered. Any registered charity can therefore bypass verification by trusted admin.

The false value returned can also be verified by the CharityRegistry.t.sol file I have written:

//SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
​
import "forge-std/Test.sol";
import "../src/CharityRegistry.sol";
​
contract CharityRegistryTest is Test {
CharityRegistry public registryContract;
address public admin;
​
function setUp() public {
admin = msg.sender;
registryContract = new CharityRegistry();
}
​
function testIsVerifiedReturnsFalseForUnverifiedCharity() public {
// Register a charity
address charity = makeAddr("charity");
registryContract.registerCharity(charity);
​
// Check that isVerified returns false
assertFalse(registryContract.isVerified(charity));
}
}
​

When this test is run with forge it fails, since it returns true

$ forge test --mt testIsVerifiedReturnsFalseForUnverifiedCharity
[⠊] Compiling...
No files changed, compilation skipped
​
Ran 1 test for test/CharityRegistry.t.sol:CharityRegistryTest
[FAIL: assertion failed] testIsVerifiedReturnsFalseForUnverifiedCharity() (gas: 33154)
Suite result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 494.01µs (204.22µs CPU time)
​
Ran 1 test suite in 7.48ms (494.01µs CPU time): 0 tests passed, 1 failed, 0 skipped (1 total tests)
​
Failing tests:
Encountered 1 failing test in test/CharityRegistry.t.sol:CharityRegistryTest
[FAIL: assertion failed] testIsVerifiedReturnsFalseForUnverifiedCharity() (gas: 33154)
​
Encountered a total of 1 failing tests, 0 tests succeeded

Note: Current test suite in the GivingThanks.t.sol file includes tests for donor function. If you only run the test testCannotDonateToUnverifiedCharity and see it reverts, and conclude "see it reverts, you cannot donate to unverified charities, it reverts because it requires charities to be verified", that would be false. If you run all tests you see GivingThanks.t.sol::testDonate() test also fails. Meaning current GivingThanks::donate() function is buggy and reverts even when it shouldn't. As it currently is no one can donate to any charity. But that's another topic.


Impact

Potentially anyone can register a charity and pass as a verified charity without actually being verified. A potentially scammy charity may still collect money from end users of the contract without being verified by the admin first. This basically removes the requirement of being verified by a trusted admin before a charity can collect money. Therefore a critical function of the protocol is nullified.

Tools Used

Foundry suite.

Recommendations

Correct the function isVerified as below to return actual verified status:

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

Lead Judging Commences

n0kto Lead Judge about 1 year 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.