First Flight #9: Soulmate
First Flight #9
Beginner FriendlyFoundryNFT
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

`Airdrop.claim()` does not check that `msg.sender` have a soulmate leads to withdraw all LoveTokens minted for `Airdrop`

Ritos

Vulnerability Details

Contract Airdrop
Function claim()
Do not check for msg.sender is in Soulmate.ownerToId mapping before calculating numberOfDaysInCouple which leads:

  • numberOfDaysInCouple equal number of days since Jan 01 1970 to block.timestamp

  • amountAlreadyClaimed equal zero.
    which leads that any account without soulmate can claim as much tokens as much days since 1970 to block.timestamp multiplied by 10 ** loveToken.decimals()

POC

Add this test to AirdropTest.t.sol and run via forge test --mt test_ClaimWithoutSoulmate() to see it success.

function test_ClaimWithoutSoulmate() public {
address lonely = makeAddr("lonely");
​
vm.warp(1707770320); // timestamp for Feb-12-2024 08:34:59 PM +UTC, 19765 days since Jan 01 1970. (UTC)
​
vm.prank(lonely);
airdropContract.claim();
​
assertTrue(loveToken.balanceOf(lonely) == 19765 ether);
}

Output:

Running 1 test for test/unit/AirdropTest.t.sol:AirdropTest
[PASS] test_ClaimWithoutSoulmate() (gas: 90054)
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 6.08ms

Tools Used

Manual review, foundry.

Recommendations

Make the following changes in Airdrop.claim()

https://github.com/Cyfrin/2024-02-soulmate/blob/b3f9227942ffd5c443ce6bccaa980fea0304c38f/src/Airdrop.sol#L51C1-L89C6

function claim() public {
+ require(soulmateContract.ownerToId(msg.sender) != 0);
// 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
);
}
Updates

Lead Judging Commences

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

finding-claim-airdrop-without-owning-NFT

High severity, This issue is separated from the flawed `isDivorced()` check presented in issue #168 as even if that is fixed, if ownership is not checked, isDivorced would still default to false and allow bypass to claim airdrops by posing as tokenId 0 in turn resulting in this [important check for token claim is bypassed.](https://github.com/Cyfrin/2024-02-soulmate/blob/b3f9227942ffd5c443ce6bccaa980fea0304c38f/src/Airdrop.sol#L61-L66). #220 is the most comprehensive issue as it correctly recognizes both issues existing within the same function.

Support

FAQs

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