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

No access control in `GivingThanks:updateRegistry` resulting anyone can alter the registry

soupy

Summary

Lack of access control in GivingThanks:updateRegistry function allows anyone to alter the registry, causing the registration and verification process of the charity in risk.

https://github.com/Cyfrin/2024-11-giving-thanks/blob/304812abfc16df934249ecd4cd8dea38568a625d/src/GivingThanks.sol#L56-L58

Vulnerability Details

There is no access control in GivingThanks:updateRegistry function which allows anyone to change the registry. This could lead to malicious registration and verification process being implemented to include non-eligible / non legitimate organzations to take part in the GivingThanks protocol

<@@>! function updateRegistry(address _registry) public {
registry = CharityRegistry(_registry);
}

Proof of Concept:

Add the following test to test\GivingThanks.t.sol

function test_audit_accessControlIssueInUpdateRegistry() public {
address randomUser = makeAddr("randomUser");
address maliciousRegistry = makeAddr("maliciousRegistry");
​
vm.prank(randomUser);
charityContract.updateRegistry(maliciousRegistry);
​
address updatedRegistry = address(charityContract.registry());
​
assertEq(updatedRegistry, maliciousRegistry);
​
}

Run the test forge test --match-test test_audit_accessControlIssueInUpdateRegistry

Ran 1 test for test/GivingThanks.t.sol:GivingThanksTest
[PASS] test_audit_accessControlIssueInUpdateRegistry() (gas: 19320)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 621.54µs (117.79µs CPU time)

The test passed indicating that anyone can freely alter the registry.

Impact

Anyone can alter the registry, enabling malicious registration and verification process being implemented to include non-eligible / non legitimate organzations to take part in the GivingThanks protocol

Tools Used

Manual review with test

Recommendations

Implement access control to allow only owner to update the registry

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

Run the test forge test --match-test test_audit_accessControlIssueInUpdateRegistry

Ran 1 test for test/GivingThanks.t.sol:GivingThanksTest
[FAIL: revert: Only owner can update registry] test_audit_accessControlIssueInUpdateRegistry() (gas: 17048)
Suite result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 592.25µs (101.79µs CPU time)

The test failed indicating that the recommened change has successfully blocked random users to update the 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.