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

[M-2] No address validation in key functions, leading to possible unexpected protocol behavior

yuza101

Summary

The key functions, CharityRegistry::registerCharity(), CharityRegistry::verifyCharity(), CharityRegistry::isVerified() & CharityRegistry::changeAdmin() does not validate input addresses, making it possible for address(0) to be used as valid input.

Vulnerability Details

  1. CharityRegistry::registerCharity()

function registerCharity(address charity) public {
registeredCharities[charity] = true;
}

  1. CharityRegistry::verifyCharity()

function verifyCharity(address charity) public {
require(msg.sender == admin, "Only admin can verify");
require(registeredCharities[charity], "Charity not registered");
verifiedCharities[charity] = true;
}

  1. CharityRegistry::isVerified()

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

  1. CharityRegistry::changeAdmin()

function changeAdmin(address newAdmin) public {
require(msg.sender == admin, "Only admin can change admin");
admin = newAdmin;
}

These functions do not validate if the charity address is address(0). Registering, verifying and changing admins with the address(0) does not make logical sense and may lead to unexpected behavior in subsequent checks or mappings. Changing the admin to address(0) for example will lock the contract permanently, all administrative functions and making the contract unmaintainable.

Impact

Allowing address(0) in these functions may lead to illogical or unexpected behavior, as address(0) should not be a valid charity or admin address.

Tools Used

Manual code review

Recommendations

Add address input validation for each function to check whether they are valid addresses and not address(0). This can be done with the require statement for example.

  1. CharityRegistry::registerCharity()

function registerCharity(address charity) public {
+ require(charity != address(0), "Invalid address");
registeredCharities[charity] = true;
}

  1. CharityRegistry::verifyCharity()

function verifyCharity(address charity) public {
+ require(charity != address(0), "Invalid address");
require(msg.sender == admin, "Only admin can verify");
require(registeredCharities[charity], "Charity not registered");
verifiedCharities[charity] = true;
}

  1. CharityRegistry::isVerified()

function isVerified(address charity) public view returns (bool) {
+ require(charity != address(0), "Invalid address");
return registeredCharities[charity];
}

  1. CharityRegistry::changeAdmin()

function changeAdmin(address newAdmin) public {
+ require(charity != address(0), "Invalid address");
require(msg.sender == admin, "Only admin can change admin");
admin = newAdmin;
}
Updates

Lead Judging Commences

n0kto Lead Judge 12 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Appeal created

n0kto Lead Judge 12 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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