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

`Airdrop::claim()` function distributes reward tokens for divorced soulmates

Summary

A soulmate who is divorced can still claim airdrop rewards.

Vulnerability Details

The Airdrop::claim() function rewards soulmates who are not divorced to claim lovetokens. The contract attempts to check the soulmates' divorce status by calling Soulmate::isDivorced():

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
);
}

If we look inside of Soulmate::isDivorced() we'll see that it checks for the divorce status for msg.sender:

function isDivorced() public view returns (bool) {
return divorced[msg.sender];
}

Since Airdrop is calling Soulmate::isDivorced(), msg.sender will represent the Airdrop contract address and not the original caller of Airdrop::claim(). This will result in Soulmate::isDivorced() to return false thus bypassing the revert.

Impact

A divorced soulmate will bypass the revert and will still be able to claim tokens.

POC

We can imagine the following scenario:

  1. User A calls Soulmate::mintSoulmateToken().

  2. User B calls Soulmate::mintSoulmateToken() and now user A & B are soulmates.

  3. User A or B calls Soulmate::getDivorced() and now user A & B are divorced.

  4. 1 day passes by.

  5. User A or B calls Airdrop::claim().

  6. Airdrop::claim() checks the divorce status of it's own contract address instead of user A & B.

  7. Airdrop::claim() rewards user A or B with tokens.

Here you can see a POC of the above scenario:

function test_bypassingDivorceCheck() public {
_mintOneTokenForBothSoulmates();
vm.prank(soulmate1);
soulmateContract.getDivorced();
vm.warp(block.timestamp + 200 days + 1 seconds);
vm.prank(soulmate1);
airdropContract.claim();
assertTrue(loveToken.balanceOf(soulmate1) == 200 ether);
vm.prank(soulmate2);
airdropContract.claim();
assertTrue(loveToken.balanceOf(soulmate2) == 200 ether);
}

Tools Used

VS Code, Foundry

Recommendations

Declare a parameter of type address called _soulmate to Soulmate::isDivorced() and check the divorce status of _soulmate instead of msg.sender:

- function isDivorced() public view returns (bool) {
+ function isDivorced(address _soulmate) public view returns (bool) {
- return divorced[_soulmate];
+ return divorced[_soulmate];
}
Updates

Lead Judging Commences

0xnevi Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-isDivorced-wrong-check

Support

FAQs

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