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

Lack of Zero Address Check in Critical Functions of GivingThanks.sol and CharityRegistry.sol

koinerdegen

Summary:

The functions in both GivingThanks.sol and CharityRegistry.sol lack checks to prevent the use of address(0). This can lead to critical issues, such as accidentally setting important addresses to address(0) or sending Ether to the zero address. It is a best practice to ensure that addresses passed to functions are not address(0).

Vulnerability Details:

Missing Zero Address Check:
The functions such as updateRegistry, donate, changeAdmin, and verifyCharity do not verify if the passed address is address(0). Using address(0) could cause unexpected behaviors, such as sending Ether to an invalid address or making an address such as the admin address invalid (address(0)).

Impact:
Medium Impact:
Failure to validate against address(0) could allow for critical issues:

  • If address(0) is used for the admin address in CharityRegistry.sol, this would break the ability to perform administrative actions.

  • If address(0) is passed as the charity address in donate, the contract would attempt to send Ether to the zero address, effectively losing the Ether.

  • The same applies to setting the registry address or charity verification, leading to loss of functionality.

Tools Used:

  • Manual Code Review

Recommendations:

  1. Create a Modifier to Check for Zero Address:
    Implement a nonZeroAddress modifier to check if the provided address is address(0) before executing the function logic.

  2. Apply the Modifier to Critical Functions:
    The nonZeroAddress modifier should be applied to all functions that involve updating addresses, such as updateRegistry, donate, changeAdmin, and any other relevant functions that deal with addresses.

  3. Update Constructors and Functions:
    Apply the check in the constructor to ensure the provided addresses are not address(0) during initialization.

Solution:

  1. Create the nonZeroAddress Modifier:

modifier nonZeroAddress(address _addr) {
require(_addr != address(0), "Address cannot be zero");
_;
}
​

** Apply the Modifier to Functions:**

Updated GivingThanks.sol:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
​
import { CharityRegistry } from "./CharityRegistry.sol";
import { ERC721URIStorage } from "@openzeppelin/contracts/token/ERC721/extensions/ERC721URIStorage.sol";
import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol";
import { Strings } from "@openzeppelin/contracts/utils/Strings.sol";
import { Base64 } from "@openzeppelin/contracts/utils/Base64.sol";
​
contract GivingThanks is ERC721URIStorage {
CharityRegistry public registry;
uint256 public s_tokenCounter;
address public immutable i_owner;
​
modifier nonZeroAddress(address _addr) {
require(_addr != address(0), "Address cannot be zero");
_;
}
​
constructor(address _registry) ERC721("DonationReceipt", "DRC") {
require(_registry != address(0), "Registry address cannot be zero");
registry = CharityRegistry(_registry);
i_owner = msg.sender;
s_tokenCounter = 0;
}
​
function donate(address charity) public payable nonZeroAddress(charity) {
require(registry.isVerified(charity), "Charity not verified");
(bool sent, ) = charity.call{value: msg.value}("");
require(sent, "Failed to send Ether");
​
_mint(msg.sender, s_tokenCounter);
​
// Create metadata for the tokenURI
string memory uri = _createTokenURI(
msg.sender,
block.timestamp,
msg.value
);
_setTokenURI(s_tokenCounter, uri);
​
s_tokenCounter += 1;
}
​
function _createTokenURI(
address donor,
uint256 date,
uint256 amount
) internal pure returns (string memory) {
// Create JSON metadata
string memory json = string(
abi.encodePacked(
'{"donor":"',
Strings.toHexString(uint160(donor), 20),
'","date":"',
Strings.toString(date),
'","amount":"',
Strings.toString(amount),
'"}'
)
);
​
// Encode in base64 using OpenZeppelin's Base64 library
string memory base64Json = Base64.encode(bytes(json));
​
// Return the data URL
return
string(
abi.encodePacked("data:application/json;base64,", base64Json)
);
}
​
function updateRegistry(address _registry) public nonZeroAddress(_registry) {
registry = CharityRegistry(_registry);
}
}
​

Updated CharityRegistry.sol:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
​
contract CharityRegistry {
address public admin;
mapping(address => bool) public verifiedCharities;
mapping(address => bool) public registeredCharities;
​
modifier nonZeroAddress(address _addr) {
require(_addr != address(0), "Address cannot be zero");
_;
}
​
constructor() {
admin = msg.sender;
}
​
function changeAdmin(address newAdmin) public nonZeroAddress(newAdmin) {
require(msg.sender == admin, "Only admin can change admin");
admin = newAdmin;
}
​
function verifyCharity(address charity) public nonZeroAddress(charity) {
require(msg.sender == admin, "Only admin can verify");
require(registeredCharities[charity], "Charity not registered");
verifiedCharities[charity] = true;
}
​
function registerCharity(address charity) public nonZeroAddress(charity) {
registeredCharities[charity] = true;
}
}
​
Updates

Lead Judging Commences

n0kto Lead Judge 7 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.