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

A user without a soulmate can receive airdrops.

SHJO

Summary

Although the protocol intends to provide airdrops only to users who have valid soulmates, a flawed method used to check numberOfDaysInCouple leads to a vulnerability where any user who does not have a soulmate can receive airdrops.

Vulnerability Details

In Airdrop::claim, the following code block is used to check the number of days in a couple.

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

The main issue is related to soulmateContract.ownerToId(msg.sender), which returns 0 when:

  • (valid) msg.sender has an NFT with an ID of 0

  • (vulnerability) msg.sender has no NFT, but 0 is returned because the default value for uint256 is 0

Therefore, any user who does not have an NFT can receive the same airdrops based on the creation timestamp corresponding to the NFT with an ID of 0. The following PoC (add this to AirdropTest.t.sol) can verify this.

function test_InvalidUserGetsAirdrop() public {
uint256 initTime = 10000;
vm.warp(initTime);
​
// soulmate 1 & 2 mint NFT
_mintOneTokenForBothSoulmates();
​
// singleSoul has no soulmate
address singleSoul = makeAddr("singleSoul");
​
// time lapse
vm.warp(initTime + 7 days);
​
// valid user
vm.prank(soulmate1);
airdropContract.claim();
​
// invalid user
vm.prank(singleSoul);
airdropContract.claim();
​
assertTrue(loveToken.balanceOf(soulmate1) == 7 ether);
assertTrue(loveToken.balanceOf(singleSoul) == 7 ether);
}

Impact

The impact of this vulnerability is HIGH. This vulnerability breaks the intended airdrop mechanism, as a user does not need to have a soulmate to receive an airdrop.

Tools Used

Foundry

Recommendations

In Airdrop::claim, check whether a user has a valid soulmate or not. A custom error error Airdrop__doesNotHaveASoulmate() could be defined and used for this purpose.

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