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

Missing access control on `GivingThanks::updateRegistry` function allows for unauthorized registry changes leading to potential exploits

phylax

Summary

The GivingThanks::updateRegistryfunction lacks proper access controls allowing any users to change the registry address to an arbitrary CharityRegistrycontract, even a malicious one.

Vulnerability Details

An attacker can deploy a malicious CharityRegistry contract that falsely verifies any address as an charity. By updating the registry they can manipulate the verification process in GivingThanks::donatefunction.

Impact

  1. The malicious registry can falsely report that a charity is verified

  2. Donors may unknowingly send funds to fraudulent charities, believing they are verified

  3. The integrity of the protocol is compromised, leading to loss of user trust

  4. The core functionality of charity verification is broken

Tools Used

Manual code review

Recommendations

Ensure the contract inherits Oppenzeppelin's Ownablecontract to utilize the onlyOwnermodifier.

import "@openzeppelin/contracts/access/Ownable.sol";
​
contract GivingThanks is ERC721URIStorage, Ownable {
// code
}

Remove owner = msg.sender;from constructor since the contract inherits Ownablecontract

constructor(address _registry) ERC721("DonationReceipt", "DRC") {
registry = CharityRegistry(_registry);
- owner = msg.sender;
tokenCounter = 0;
}

Use the onlyOwnermodifier in updateRegistryso only the owner can call the function and check for zero address.

function updateRegistry(address _registry) public onlyOwner {
require(_registry != address(0), "Invalid registry address");
registry = CharityRegistry(_registry);
}

For additional measures the function could emit an event when registry is updated to enhance transparency.

+ event RegistryUpdated(address indexed newRegistry);
​
function updateRegistry(address _registry) public onlyOwner {
require(_registry != address(0), "Invalid registry address");
registry = CharityRegistry(_registry);
+ emit RegistryUpdated(_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.