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

'Airdrop.sol::claim' does not correctly check if the address claiming has a soulmate

coffee

Summary

As per the documentation, only people who have a soulmate should be able to claim the airdrop.

Vulnerability Details

In 'Airdrop.sol::claim' numberOfDaysInCouple is not calculated correctly to assure that only people with a soulmate can collect the airdrop.

uint256 numberOfDaysInCouple = (block.timestamp - soulmateContract.idToCreationTimestamp(soulmateContract.ownerToId(msg.sender))) daysInSecond;

When getting the idToCreationTimestamp of the ownerToId of the msg.sender, this logic would return 0 when the address claiming has not tried to mint a soulmate token or if they are waiting for a soulmate. This would result in the problem below:

(block.timestamp - 0) / daysInSecond;

Meaning the address could claim as many loveTokens as time has passed on the Ethereum blockchain.

Impact

This test passes showing that anyone that does not have a soulmate can claim the airdrop

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

Tools Used

--Foundry

Recommendations

It is recommended add a check to make sure the msg.sender has a soulmate

function claim() public {
// No LoveToken for people who don't love their soulmates anymore.
if (soulmateContract.isDivorced()) revert Airdrop__CoupleIsDivorced();
​
+ if (soulmateContract.soulmateOf(msg.sender) == address(0)) {
+ revert Airdrop__DoesNotHaveASoulmate();
+ }
​
// Calculating since how long soulmates are reunited
uint256 numberOfDaysInCouple = (block.timestamp -
soulmateContract.idToCreationTimestamp(
soulmateContract.ownerToId(msg.sender)
)) / daysInSecond;
​
uint256 amountAlreadyClaimed = _claimedBy[msg.sender];
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.