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

Lack of Access Control in `updateRegistry`

abhishekthakur

Summary

The updateRegistry function lacks access control, allowing anyone to change the registry address and potentially redirect donations to unverified charities.

Vulnerability Details

https://github.com/Cyfrin/2024-11-giving-thanks/blob/main/src/GivingThanks.sol#L56-L58

function updateRegistry(address _registry) public {
registry = CharityRegistry(_registry); // No access control
}

Due to lack of access control on above function, anybody can update the registry, that will give an oppurtunity to the attacker to verify any registry and take all the donations.

POC

function testUnauthorizedRegistryUpdate() public {
address attacker = makeAddr("attacker");
CharityRegistry maliciousRegistry = new CharityRegistry();
vm.prank(attacker);
charityContract.updateRegistry(address(maliciousRegistry));
// Attacker successfully changed registry
assertEq(address(charityContract.registry()), address(maliciousRegistry));
}

Impact

  • Attackers can change registry to malicious contract

  • Can bypass charity verification

  • Can redirect donations to unverified addresses

Tools Used

Manual Review, Foundry

Recommendations

Add an access control will fix the issue.

function updateRegistry(address _registry) public {
require(msg.sender == owner, "Only owner can update registry");
registry = CharityRegistry(_registry);
}
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.