Raisebox Faucet
First Flight #50
Beginner FriendlySolidity
100 EXP
View results
Previous Next
Submission Details
Severity: medium
Valid

[M-3] ETH and token daily limits use different reset logic, enabling limit bypass

cyberethdev

[M-3] ETH and token daily limits use different reset logic, enabling limit bypass

Description

  • Expected bahaviour There should be a consistent calculation of "days" and time passed when calculating the number of total token and ETH claims.

  • Problematic bahaviour
    ETH claims (dailyDrips): counts days according to how many 24 hour periods have elapsed since the first Unix timestamp.
    Token claims (dailyClaimCount): counts days as rolling 24 hour windows based on the timestamp of the last token claim.

.
.
.
@> uint256 currentDay = block.timestamp / 24 hours;
@> if (currentDay > lastDripDay) {
lastDripDay = currentDay;
dailyDrips = 0;
// dailyClaimCount = 0;
}
.
.
.
@> if (block.timestamp > lastFaucetDripDay + 1 days) {
@> lastFaucetDripDay = block.timestamp;
dailyClaimCount = 0;
}
.
.
.

Risk

Likelihood: High

  • The binning of token and ETH claims in a given time window occurs whenever the claimFaucetTokens is called.

Impact: Low

  • This incosistency can lead to incosistent ETH and token claim counts per day and a bypass of the set thresholds.

Proof of Concept

Add the following test in the Foundry test suite and run with forge test --mt test_claimFaucetTokens_IncosistentTimeCalculation.

  1. Time is set to 5 minutes before midnight.

  2. User1 claims tokens --> 1 token claim and one ETH claim have been processed.

  3. Time is set to 10 minutes after midnight.

  4. User2 claims tokens --> total number of token claims is now 2 while the number of ETH drips is only 1 (ETH counter reset at midnight)

function test_claimFaucetTokens_IncosistentTimeCalculation() public {
uint256 dayInSecs = 24 * 60 * 60;
uint256 fiveMinsInSecs = 5 * 60;
uint256 tenMinsInSecs = 10 * 60;
uint256 startDay = 4 * dayInSecs;
​
// First-time user claim tokens 5 minutes before midnight
vm.warp(startDay - fiveMinsInSecs);
​
vm.prank(user1);
raiseBoxFaucet.claimFaucetTokens();
​
// Count of total ETH drips before midnight
uint256 dailyEthDripsAmountBeforeMidnight = raiseBoxFaucet.dailyDrips();
uint256 dailyEthDripsCountBeforeMidnight = dailyEthDripsAmountBeforeMidnight / raiseBoxFaucet.sepEthAmountToDrip();
// Count of total token claims before midnight
uint256 dailyTokenClaimsBeforeMidnight = raiseBoxFaucet.dailyClaimCount();
​
// Check that 1 ETH drip has been dispensed and 1 token claim
assertTrue(dailyEthDripsCountBeforeMidnight == dailyTokenClaimsBeforeMidnight);
​
// Fast-forward 10 mins to after midnight
vm.warp(startDay + tenMinsInSecs);
​
// User2 claims tokens
vm.prank(user2);
raiseBoxFaucet.claimFaucetTokens();
​
// Count of total ETH drips after midnight
uint256 dailyEthDripsAmountAfterMidnight = raiseBoxFaucet.dailyDrips();
uint256 dailyEthDripsCountAfterMidnight = dailyEthDripsAmountAfterMidnight / raiseBoxFaucet.sepEthAmountToDrip();
// Count of total token claims after midnight
uint256 dailyTokenClaimsAfterMidnight = raiseBoxFaucet.dailyClaimCount();
​
// Assert that the number of ETH drips is lower than the number of token claims
assertTrue(dailyEthDripsCountAfterMidnight < dailyTokenClaimsAfterMidnight);
assertTrue(dailyTokenClaimsAfterMidnight == 2);
}
​

Recommended Mitigation

Use a consistent time measurement by binning the claims in their respective 24 hour window as is done for the dailyDrips variable:

function claimFaucetTokens() public {
​
+ uint256 currentDay = block.timestamp / 24 hours;
+ if (currentDay > lastDripDay) {
+ lastDripDay = currentDay;
+ dailyDrips = 0;
+ dailyClaimCount = 0;
+ }
faucetClaimer = msg.sender;
​
// (lastClaimTime[faucetClaimer] == 0);
​
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();
}
​
// still checks
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;
}
​
/**
*
* @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;
- }
​
// Effects
​
lastClaimTime[faucetClaimer] = block.timestamp;
dailyClaimCount++;
​
// Interactions
_transfer(address(this), faucetClaimer, faucetDrip);
​
emit Claimed(msg.sender, faucetDrip);
}

There is also no need to use to different variables for tracking the last drip day.
Simplifiy the following variables:
lastDripDay, lastFaucetDripDay
to one unified variable: lastClaimDay.

Updates

Lead Judging Commences

inallhonesty Lead Judge 7 days ago
Submission Judgement Published
Validated
Assigned finding tags:

Inconsistent day calculation methods cause desynchronization between ETH and token daily resets.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.