dailyClaimLimit reset is after the require dailyClaimCount < dailyClaimLimit
Description
-
Normally, dailyClaimCount should reset after 24 hours have passed. If 24 hours have not yet passed, the reset is skipped.
Immediately after that, the function performs the check dailyClaimCount < dailyClaimLimit to ensure it can still be used.
Before the end of the function, dailyClaimCount is incremented by 1 to keep track of the number of times the function has been called.
-
Every time dailyClaimCount reaches dailyClaimLimit, the claimFaucetToken function becomes unusable because it will always revert at the dailyClaimCount >= dailyClaimLimit check.
The part of the function where dailyClaimCount is reset after 24 hours is placed **after **this check so that the reset will never happen because function revert before the reset.
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:
Impact:
-
Users can't use the main function claimFaucetTokens, the contract will be useless
-
Thanks to the function that allows owner to increase the dailyClaimLimit, the contract is not broken forever. Anyway, everytime dailyClaimLimit is reached, owner needs to call adjustDailyClaimLimit function to unlock the contract.
Proof of Concept
The PoC demonstrates that once the daily limit is reached, even after 24 hours have passed, the counter is not reset when calling the claimFaucetTokens function, causing the transaction to revert. The test also shows that once the owner increases the limit, the function resumes working as expected.
function testIfDOSHappenDueToDailyClaimCountEqualDailyClaimLimit() public {
vm.prank(owner);
raiseBoxFaucet.adjustDailyClaimLimit(98, false);
vm.prank(user1);
raiseBoxFaucet.claimFaucetTokens();
vm.prank(user2);
raiseBoxFaucet.claimFaucetTokens();
vm.prank(user3);
vm.expectRevert();
raiseBoxFaucet.claimFaucetTokens();
vm.warp(block.timestamp + 25 hours);
vm.prank(user3);
vm.expectRevert();
raiseBoxFaucet.claimFaucetTokens();
assertEq(raiseBoxFaucet.dailyClaimCount(), 2);
assertEq(raiseBoxFaucet.dailyClaimCount(), raiseBoxFaucet.dailyClaimLimit());
vm.prank(owner);
raiseBoxFaucet.adjustDailyClaimLimit(10, true);
vm.prank(user3);
raiseBoxFaucet.claimFaucetTokens();
assertEq(raiseBoxFaucet.dailyClaimCount(), 1);
}
Recommended Mitigation
Move the part of the code that checks whether a day has passed — and resets the counter if so — to before the check that compares dailyClaimCount with dailyClaimLimit.
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 (block.timestamp > lastFaucetDripDay + 1 days) {
+ lastFaucetDripDay = block.timestamp;
+ dailyClaimCount = 0;
+ }
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;
}
- if (block.timestamp > lastFaucetDripDay + 1 days) {
- lastFaucetDripDay = block.timestamp;
- dailyClaimCount = 0;
- }
// Effects
lastClaimTime[faucetClaimer] = block.timestamp;
dailyClaimCount++;
// Interactions
_transfer(address(this), faucetClaimer, faucetDrip);
emit Claimed(msg.sender, faucetDrip);
}
