Beginner FriendlyFoundryNFT
100 EXP
View results
Submission Details
Severity: high
Invalid

Bad implementations in claimer Access Control for `Airdrop::claim()` function

Summary

In this case, _claimedBy” is never set to true, so the claimant can issue call the function multiple times. msg.senderdoes not have adequate restrictions. So ,claim` function can be called called multiples times by the soulmates and it can ruin its functionality.

Vulnerability Details

Airdrop contract define aclaim() function that is protected by a _claimedBy

function claim() public {
// No LoveToken for people who don't love their soulmates anymore.
if (soulmateContract.isDivorced()) revert Airdrop__CoupleIsDivorced();
// Calculating since how long soulmates are reunited
uint256 numberOfDaysInCouple = (block.timestamp -
soulmateContract.idToCreationTimestamp(
soulmateContract.ownerToId(msg.sender)
)) / daysInSecond;
uint256 amountAlreadyClaimed = _claimedBy[msg.sender];
if (
amountAlreadyClaimed >=
numberOfDaysInCouple * 10 ** loveToken.decimals()
) revert Airdrop__PreviousTokenAlreadyClaimed();
uint256 tokenAmountToDistribute = (numberOfDaysInCouple *
10 ** loveToken.decimals()) - amountAlreadyClaimed;
// Dust collector
if (
tokenAmountToDistribute >=
loveToken.balanceOf(address(airdropVault))
) {
tokenAmountToDistribute = loveToken.balanceOf(
address(airdropVault)
);
}
_claimedBy[msg.sender] += tokenAmountToDistribute;
emit TokenClaimed(msg.sender, tokenAmountToDistribute);
loveToken.transferFrom(
address(airdropVault),
msg.sender,
tokenAmountToDistribute
);
}
}

However, In the above case, the modifier implementation is flawed as there isn’t any check for a require or revert,
the comparison will silently return false and let the execution continue:

modifier _claimedBy() {
msg.sender == owner;
_;
}

Impact

Any account can claim tokens, any number of Lovetokens.

This represents a critical issue as the Soulmate NFT token can be used to claim 1 LoveToken per day. An attacker can freely claim Lovetokens to steal all the rewards from it.

Tools Used

Manual Review

Recommendations

The modifier should require that the caller is the _claimedBy in order to revert the call in case this condition doesn’t hold.

Updates

Lead Judging Commences

0xnevi Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Other

Support

FAQs

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