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

Users without a soulmate can claim a large amount of airdrop tokens

ceseshi

Summary

The Airdrop contract allows users without a soulmate to claim a large amount of tokens, which should not be possible.

Vulnerability Details

The claim() function in Airdrop.sol does not check if the user has a soulmate, so the calculation takes a wrong number of days and sends and incorrect amount of tokens to the user.

This test calls claim() without a soulmate.

function testClaimAirdropWithoutSoulmate() public {
// Jump to an actual timestamp in mainnet
uint256 timestamp = 1707775379;
vm.warp(timestamp);
​
// Get a new user
address alice = makeAddr("alice");
​
// Claim
vm.prank(alice);
airdropContract.claim();
​
console2.log("balance", loveToken.balanceOf(alice));
}

The test shows the tokens sent to alice.

Running 1 test for test/unit/AuditTest1.t.sol:AuditTest1
[PASS] testClaimAirdropWithoutSoulmate() (gas: 93299)
Logs:
balance 19765000000000000000000

Impact

Users without a soulmate can steal tokens from the airdrop vault.

Tools Used

Foundry, Manual review

Recommendations

Add a soulmate check in Airdrop:claim()

+error Airdrop__NoSoulmate();
​
function claim() public {
// No LoveToken for people who don't love their soulmates anymore.
+ if (soulmateContract.soulmateOf(msg.sender) == address(0)) revert Airdrop__NoSoulmate();
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];
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.