[H-2] dailyDrips falsely resets to 0, allowing the contract's sepolia ETH funds to be drained faster than intended
Description
-
Expected bahaviour The total sepolia ETH dripped by the faucet in one day should not surpass the threshold defined in the state variable dailySepEthCap.
-
Problematic bahaviour When non-first time users claim tokens, the dailyDrips state variable falsely resets to 0. By resetting dailyDrips to zero, the faucet can be manipulated to send more sep ETH in a day than the maximum daily ETH amount set in dailySepEthCap.
function claimFaucetTokens() public {
.
.
.
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;
@> }
.
.
.
}
Risk
Likelihood: High
Impact: Medium - High
This vulnerability allows the contract's sepolia ETH funds to be drained faster than intended. The severity of the drain depends on the dailyClaimLimit. The higher the daily claim limit is, the more sepolia ETH funds can be drained from the contract, potentially allowing a full drain within a day.
Proof of Concept
As a PoC add the following test to the Foundry test suite and run with forge test --mt test_dailyDrips_FalselyResets_AllowingMoreSepETHDrips.
Hypothetical Scenario
Day 1: Owner increases the dailyClaimLimit to 200.
The maximum number of sepolia eth drips that can be processed are 100, based on the contract's set up.
First time user claims tokens.
Cooldown passes (3 days)
Day 5: 100 first time users claim tokens and receive ETH. The max sep ETH limit has been reached.
A second-time user claims tokens again --> dailyDrips is reset from 0.5 ETH to 0 ETH.
Another 99 first time users claim tokens and receive ETH. Total SEP ETH sent is higher than the 0.5 ETH limit.
function test_dailyDrips_FalselyResets_AllowingMoreSepETHDrips() public {
vm.prank(owner);
raiseBoxFaucet.adjustDailyClaimLimit(100, true);
assertTrue(raiseBoxFaucet.dailyClaimLimit() == 200, "dailyClaimLimit is not 200");
uint256 maxSepEthDailyClaims = raiseBoxFaucet.dailySepEthCap() / raiseBoxFaucet.sepEthAmountToDrip();
assertTrue(maxSepEthDailyClaims == 100, "The maximum number of allowed daily sep ETH drips is not 100");
address newUser = makeAddr("newUser");
vm.prank(newUser);
raiseBoxFaucet.claimFaucetTokens();
vm.warp(block.timestamp + raiseBoxFaucet.CLAIM_COOLDOWN() + 1 hours);
uint256 totalEthDripped;
for(uint256 i=0; i < 100; i++) {
string memory claimer = string.concat("user",vm.toString(i));
address user = makeAddr(claimer);
vm.prank(user);
raiseBoxFaucet.claimFaucetTokens();
totalEthDripped += raiseBoxFaucet.sepEthAmountToDrip();
}
assertTrue(totalEthDripped == raiseBoxFaucet.dailySepEthCap(), "ETH drips have not reached the limit");
assertTrue(raiseBoxFaucet.getHasClaimedEth(newUser), "newUser is a first time user");
vm.prank(newUser);
raiseBoxFaucet.claimFaucetTokens();
assertTrue(raiseBoxFaucet.dailyDrips() == 0, "Daily drips value is not zero");
for(uint256 i=100; i < 199; i++) {
string memory claimer = string.concat("user",vm.toString(i));
address user = makeAddr(claimer);
vm.prank(user);
raiseBoxFaucet.claimFaucetTokens();
}
totalEthDripped += raiseBoxFaucet.dailyDrips();
assertTrue(totalEthDripped > raiseBoxFaucet.dailySepEthCap(), "The daily sep ETH cap has not been exceeded");
assertTrue(address(raiseBoxFaucet).balance == 0, "Sep ETH balance is not 0");
}
Recommended Mitigation
To mitigate this vulnerability remove the else block that resets dailyDrips to zero:
function claimFaucetTokens() public {
.
.
.
if (!hasClaimedEth[faucetClaimer] && !sepEthDripsPaused) {
uint256 currentDay = block.timestamp / 24 hours;
if (currentDay > lastDripDay) {
lastDripDay = currentDay;
dailyDrips = 0;
// dailyClaimCount = 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;
- }
.
.
.
}
The resets of dailyDrips and dailyClaimCount should also use a consistent calculation of days passed, which will be addressed as a separate finding.