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

Missing Access Control in updateRegistry Function

sauronsol

Summary

The updateRegistry function lacks access control, allowing any address to change the charity registry address, effectively compromising the entire donation system.

Vulnerability Details

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

Test demonstrating vulnerability:

function testUpdateRegistryNoAccessControl() public {
address attacker = makeAddr("attacker");
address maliciousRegistry = makeAddr("maliciousRegistry");
vm.startPrank(attacker);
charityContract.updateRegistry(maliciousRegistry);
assertEq(address(charityContract.registry()), maliciousRegistry);
vm.stopPrank();
}

Impact

HIGH severity:

  • Any address can change the registry

  • Attacker can point to malicious registry that validates fake charities

  • Complete compromise of donation verification system

  • Potential theft of donations through fake charities

Tools Used

  • Manual code review

  • Foundry testing framework

  • Custom access control test

Recommendations

  1. Add Ownable pattern:

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

  1. Or implement role-based access control:

contract GivingThanks is ERC721URIStorage, AccessControl {
bytes32 public constant ADMIN_ROLE = keccak256("ADMIN_ROLE");
​
constructor() {
_grantRole(ADMIN_ROLE, msg.sender);
}
​
function updateRegistry(address _registry) public onlyRole(ADMIN_ROLE) {
require(_registry != address(0), "Invalid registry address");
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.