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

any user can claim airdrops regardless of having a soulmate NFT

Summary

The Airdrop::claim function insufficiently checks, if the msg.sender holds a soulmate NFT, leading to any user, regardless of having a soulmate or looking for a soulmate, being able to claim rewards. Due to this issue, the calculation of how long the soulmates are reunited fails and pays out incorrect amounts of tokens.

Vulnerability Details

The claim function does not check, if the person claiming has the soulmate NFT. Due to this, any person can call claim and receive airdrops. Moreover, the function to calculate the rewards fails due to the msg.sender not having a soulmate:

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

Due to not having a soulmate soulmateContract.ownerToId(msg.sender) returns Id: 0, which then gets passed into soulmateContract.idToCreationTimestamp, which returns the timestamp of the very first soulmate reunion (or 0, if there are currently no soulmate couples). This leads to the caller receiving airdrops, as if they were reunited as long as the first couple in the contract.

Run the following PoC in baseTest.t.sol to see the vulnerability:

function testUserWithoutSoulmateCanClaimAirdrop() external {
// just claim
vm.warp(block.timestamp + 1 weeks);
vm.prank(soulmate1);
airdropContract.claim();
console.log("tokens claimed: ", loveToken.balanceOf(soulmate1));
}

Impact

Anyone can claim rewards, even if hey have no soulmate (or aren't even signed up to look for a soulmate).

Tools Used

Foundry

Recommendations

Appropriately check for the msg.sender soulmate NFT balance and revert, if they do not possess a soulmate NFT.

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.