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

Lack of `Access Control` on `updateRegistry`

freesultan

Summary

In the code below, the updateRegistry function is public and lacks access control, allowing anyone to call it:

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

Vulnerability Details

In the GivingThanks contract, there is a function for updating the registry address that lists charities, as shown in the code below:

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

In this code, anyone can call updateRegistry and change the registry to their preferred registry, potentially one they own.

Impact

  • Since only one registry of charities can be set at a time in the GivingThanks contract, allowing anyone to alter this registry undermines the contract’s intended purpose.

  • This vulnerability enables any user to replace the verified charities list with their own.

  • The likelihood of this function being called by others is high.

Tools Used

Remix, Manual Review, Slither, Aderyn

Recommendations

To address this, add ownership verification within the function, either through explicit code or by using OpenZeppelin’s onlyOwner modifier.

Implementing Ownership Control:

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

OpenZeppelin:

- function updateRegistry(address _registry) public {
+ function updateRegistry(address _registry) public onlyOwner {
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.