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

Missing validation in the `Airdrop.sol::claim()` function allows users without soulmates to claim airdrop tokens

stefanlatinovic

Summary

Missing validation checks in the Airdrop.sol::claim() function will allow users without soulmates to claim airdrop tokens

Vulnerability Details

In the Airdrop.sol::claim() function, there is a code block that calculates the number of days a couple has been soulmates:

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

This number is later used to determine the number of tokens that should be distributed to the caller.

soulmateContract.idToCreationTimestamp() will return 0 if the caller initiates Soulmate.sol::mintSoulmateToken() but is still waiting for a soulmate to be assigned.
soulmateContract.idToCreationTimestamp() will return the creation timestamp of the token with id 0 if the caller didn't initiate Soulmate.sol::mintSoulmateToken().

In the end, this vulnerability has the potential to cause a drain of funds from the airdrop vault.

Impact

Users without soulmates can claim airdrop tokens.

Proof of Concept (PoC)

Add the following test in AirdropTest.t.sol:

function test_usersWithoutSoulmatesCanClaimAirdropTokens(address random) public {
// Random address validation
vm.assume(random != address(soulmateContract) && random != address(loveToken) &&
random != address(stakingContract) && random != address(airdropContract) &&
random != address(airdropVault) && random != address(stakingVault)
);
​
assertEq(soulmateContract.soulmateOf(random), address(0)); // user without soulmate
uint256 airdropVaultTokenBalanceBeforeClaim = loveToken.balanceOf(address(airdropVault));
uint256 userTokenBalanceBeforeClaim = loveToken.balanceOf(random);
assertEq(userTokenBalanceBeforeClaim, 0);
​
uint256 mockedBlockTimestamp = 1707973127; // timestamp of Ethereum block number `19231100`
vm.warp(mockedBlockTimestamp);
vm.prank(random);
airdropContract.claim(); // users without soulmates are allowed to claim airdrop tokens
​
uint256 airdropVaultTokenBalanceAfterClaim = loveToken.balanceOf(address(airdropVault));
uint256 userTokenBalanceAfterClaim = loveToken.balanceOf(random);
​
assertGt(userTokenBalanceAfterClaim, userTokenBalanceBeforeClaim);
assertEq(airdropVaultTokenBalanceAfterClaim, airdropVaultTokenBalanceBeforeClaim - userTokenBalanceAfterClaim);
}

Run a test with forge test --mt test_usersWithoutSoulmatesCanClaimAirdropTokens.

Tools Used

  • Manual review

  • Foundry

Recommendations

The transaction should be reverted if the sender doesn't have a soulmate.

Recommended changes to the Airdrop.sol::claim() function:

/*//////////////////////////////////////////////////////////////
ERRORS
//////////////////////////////////////////////////////////////*/
error Airdrop__CoupleIsDivorced();
error Airdrop__PreviousTokenAlreadyClaimed();
+error Airdrop__UsersWithoutSoulmateCannotClaimAirdropTokens();
​
function claim() public {
+ if (soulmateContract.soulmateOf(msg.sender) == address(0)) {
+ revert Airdrop__UsersWithoutSoulmateCannotClaimAirdropTokens();
+ }
​
// 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
);
}

Add the following import and test in AirdropTest.t.sol:

import {Airdrop} from "../../src/Airdrop.sol";
​
function test_claimRevertsWhenCallerIsWithoutSoulmate(address random) public {
// Random address validation
vm.assume(random != address(soulmateContract) && random != address(loveToken) &&
random != address(stakingContract) && random != address(airdropContract) &&
random != address(airdropVault) && random != address(stakingVault)
);
​
assertEq(soulmateContract.soulmateOf(random), address(0)); // user without a soulmate
​
uint256 mockedBlockTimestamp = 1707973127; // timestamp of Ethereum block number `19231100`
vm.warp(mockedBlockTimestamp);
vm.expectRevert(abi.encodeWithSelector(Airdrop.Airdrop__UsersWithoutSoulmateCannotClaimAirdropTokens.selector)); // users without soulmates can't claim airdrop tokens
vm.prank(random);
airdropContract.claim();
}

Run a test with forge test --mt test_claimRevertsWhenCallerIsWithoutSoulmate.

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.