donate
function in the GivingThanks.sol
contract is vulnerable to a reentrancy attack due to the external call to charity.call{value: msg.value}("")
. This call is made before the state variables are updated, enabling a malicious charity contract to re-enter the function and perform additional malicious actions before the state is fully updated.charity
address (which could be a contract) and then performs state-changing actions such as minting a token and updating the metadata. If the charity
contract is malicious and contains a fallback function, it could re-enter the donate
function before the state changes, allowing an attacker to exploit the situation (e.g., by calling the donate
function repeatedly and draining Ether).Here is the vulnerable part of the code:
donate
function can result in a reentrancy attack. This could allow a malicious contract to repeatedly call donate
, draining the contract's funds and potentially minting multiple tokens for the same donation. This poses a serious risk to the integrity and security of the contract and its funds.Manual code review
Slither (for vulnerability detection)
Use the Checks-Effects-Interactions Pattern: Move all state changes (such as minting the token, setting the URI, and updating tokenCounter
) before the external call to ensure that the contract state is updated before the external contract can interfere.
Use a ReentrancyGuard
Modifier: Implement the nonReentrant
modifier from OpenZeppelin’s ReentrancyGuard
to prevent multiple calls to the donate
function during execution.
Explanation of the Fixes
ReentrancyGuard
: The nonReentrant
modifier prevents reentrancy by blocking any re-entry into the donate
function. This ensures that once the function is entered, no other calls can interrupt it until it completes.
Checks-Effects-Interactions Pattern: All internal state changes (such as minting the token, setting the token URI, and incrementing the tokenCounter
) are now done before the external call to charity
. This prevents any reentrancy issues, as the contract’s state is fully updated before the external call is made.
By applying both the ReentrancyGuard
and the Checks-Effects-Interactions pattern, the reentrancy vulnerability in the donate
function is mitigated. These changes secure the contract by ensuring that external calls cannot interfere with the contract’s state, protecting it from malicious reentrancy attacks.
Impact: High, one charity can reenter the donate function with the same ETH provided and mint several NFT. Likelyhood: Low, any malicious charity can do it but Admin is trusted and should verify the charity contract before "verifying" it.
The contest is live. Earn rewards by submitting a finding.
This is your time to appeal against judgements on your submissions.
Appeals are being carefully reviewed by our judges.