一

Summary

The attacker can register their own address as the charitable organization and then perform a self-transfer by donating to the organization, effectively receiving a specific minted NFT for free.

Vulnerability Details

Vulnerability code source:
https://github.com/Cyfrin/2024-11-giving-thanks/blob/304812abfc16df934249ecd4cd8dea38568a625d/src/GivingThanks.sol#L21-L23

There is no check to verify whether the charitable organization’s address is the same as the donor’s address, which allows attackers to mint specific NFTs for free. In other words, the registration requirements for a charitable organization are too lenient — only the admin’s approval is needed. This creates a situation where an attacker can effectively perform a self-transfer, resulting in the acquisition of more NFTs.

Impact

The attacker can disguise their own address as a charitable organization, obtain the admin’s confirmation, and then call the donate function, passing their own address as the charitable organization’s address. This allows the attacker to donate tokens and receive a specific NFT. If the attacker repeats the donation multiple times, it will lead to the minting of many NFTs, resulting in resource wastage and potentially the destruction of NFTs. If possible, the attacker may also sell the NFTs, gaining illegal profits.If this vulnerability is exploited by an attacker, it would undermine the reward mechanism for donors, allowing malicious individuals who haven’t donated to also receive specific NFTs.

Tools Used

Manual review.

Recommendations

Add a require condition to check if the charitable organization’s address is the same as the donor’s address. If they are the same, prevent the donation from being processed.

function donate(address charity) public payable {
        require(registry.isVerified(charity), "Charity not verified");

       require(msg.sender == charity,"Not the correct charitable organization“）

        (bool sent,) = charity.call{value: msg.value}("");
        require(sent, "Failed to send Ether");

        _mint(msg.sender, tokenCounter);
//...
}

二

Summary

Anyone can modify the registry, leading to a shift in power, which could allow attackers to freely register charitable organizations.

Vulnerability Details

The source of the vulnerability code:
https://github.com/Cyfrin/2024-11-giving-thanks/blob/304812abfc16df934249ecd4cd8dea38568a625d/src/GivingThanks.sol#L56-L58

Detailed vulnerability code.

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

This function is public, meaning anyone can modify the registry. In other words, an attacker could register themselves as the registry and then provide the address of a charitable organization to register it. Only the admin is supposed to confirm the registration, but since the attacker has now become the admin, this creates a vulnerability that could be exploited for uncontrollable attacks.

Impact

An attacker can call the updateRegistry public function, passing their own address as a parameter to change the registry. Then, by calling the donate function again, the attacker could either pass their own address or another address, allowing them to illegally receive donated tokens and engage in malicious fundraising.

Tools Used

Manual review.

Recommendations

Add a require condition in theupdateRegistryfunction to restrict who can modify the registry.

function updateRegistry(address _registry) public {
      require(msg.sender == owner, "NOT TURE ADMIN"
       registry = CharityRegistry(_registry);
   }

这次的报告，只有第一个被视为有效，还需要学习