Root + Impact
Description
A reentrancy vulnerability exists in RaiseBoxFaucet.claimFaucetTokens() where the contract executes an external faucetClaimer.call{value: ...}("") before finalizing critical state updates and token transfers, allowing a malicious contract to reenter and double-claim tokens.
function claimFaucetTokens() public {
faucetClaimer = msg.sender;
...
if (!hasClaimedEth[faucetClaimer] && !sepEthDripsPaused) {
...
if (dailyDrips + sepEthAmountToDrip <= dailySepEthCap && address(this).balance >= sepEthAmountToDrip) {
hasClaimedEth[faucetClaimer] = true;
dailyDrips += sepEthAmountToDrip;
@> (bool success, ) = faucetClaimer.call{value: sepEthAmountToDrip}("");
@>
@>
@>
if (success) {
emit SepEthDripped(faucetClaimer, sepEthAmountToDrip);
} else {
revert RaiseBoxFaucet_EthTransferFailed();
}
} else {
emit SepEthDripSkipped(...);
}
}
...
@> lastClaimTime[faucetClaimer] = block.timestamp;
@> dailyClaimCount++;
@> _transfer(address(this), faucetClaimer, faucetDrip);
emit Claimed(msg.sender, faucetDrip);
}
Risk
Likelihood: HIGH
-
The vulnerability occurs whenever a malicious smart contract interacts with claimFaucetTokens(), since the function performs an external ETH transfer (.call{value: ...}) before updating critical state variables.
-
Attacker only needs to call claimFaucetTokens() through a contract with a fallback function to reenter.
Impact: HIGH / CRITICAL
-
A successful reentrancy allows an attacker to bypass claim limits and cooldowns, repeatedly claiming tokens and ETH within a single transaction.
-
This leads to direct financial loss of faucet assets and broken accounting logic.
Proof of Concept:
A malicious contract’s receive() reenters claimFaucetTokens() while the faucet still holds pre‑update state because the faucet does an external .call{value:...} before finalizing state and the ERC‑20 transfer. Each reentrant call meets the original checks and collects another drip, enabling multiple token/ETH drips in one transaction.
function testReentrancyExploitSucceeds() public {
uint256 faucetDrip = faucet.faucetDrip();
console.log("Faucet drip (tokens):", faucetDrip);
uint256 faucetInitialTokenBalance = faucet.balanceOf(address(faucet));
assertTrue(faucetInitialTokenBalance >= faucetDrip, "faucet has insufficient tokens for drip");
vm.prank(attackerEOA);
attacker.setMaxReentries(3);
vm.prank(attackerEOA);
attacker.attack{value: 0}(address(faucet));
uint256 attackerTokenBal = faucet.balanceOf(address(attacker));
console.log("Attacker token balance after attack:", attackerTokenBal);
assertEq(attackerTokenBal > faucetDrip, "Reentrancy exploit did NOT increase attacker balance above single drip");
uint256 faucetFinalTokenBalance = faucet.balanceOf(address(faucet));
console.log("Faucet token balance after attack:", faucetFinalTokenBalance);
assertEq(faucetFinalTokenBalance < faucetInitialTokenBalance - faucetDrip, "Faucet did not lose additional tokens as expected from reentrancy");
}
}
Recommended Mitigation
The recommended code changes eliminate the reentrancy window and provide layered protection:
1.Checks → Effects → Interactions
2.Adding OpenZeppelin’s ReentrancyGuard and marking claimFaucetTokens() nonReentrant
- (bool success,) = faucetClaimer.call{value: sepEthAmountToDrip}("");
-
- if (success) {
- emit SepEthDripped(faucetClaimer, sepEthAmountToDrip);
- } else {
- revert RaiseBoxFaucet_EthTransferFailed();
- }
+ // Follow the Checks-Effects-Interactions pattern
+ // Move all state updates before any external call to prevent reentrancy.
+ lastClaimTime[faucetClaimer] = block.timestamp;
+ dailyClaimCount++;
+
+ // Use `Address.sendValue` (OpenZeppelin) instead of low-level call
+ // to safely forward ETH without reentrancy risk.
+ (bool success, ) = payable(faucetClaimer).call{value: sepEthAmountToDrip}("");
+ require(success, "RaiseBoxFaucet_EthTransferFailed");
+
+ // After successful transfer, emit the event
+ emit SepEthDripped(faucetClaimer, sepEthAmountToDrip);
- _transfer(address(this), faucetClaimer, faucetDrip);
+ // Consider adding a ReentrancyGuard to the contract for layered protection
+ // and ensuring no function performs external calls before updating state.
+ _transfer(address(this), faucetClaimer, faucetDrip);
