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

Any user can claim airdrop rewards via `Airdrop::claim()` resulting in a loss of funds

glitchical

Summary

The Airdrop::claim() function distributes tokens to any caller, even to those awaiting a soulmate, or have never minted a soulmate.

Vulnerability Details

The Airdrop::claim() function is meant to give tokens to soulmates, however any user, even those who don't have a soulmate or never minted a soulmate via the function Soulmate::mintSoulmateToken() can claim rewards. This can happen due to setting the numberOfDaysInCouple local variable:

function claim() public {
// 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
);
}

When numberOfDaysInCouple is set, it initally calls Soulmate::ownerToId() and passes msg.sender as the argument. This mapping maps the address to a uint256:

mapping(address owner => uint256 id) public ownerToId;

Initially this mapping is set inside of Soulmate::mintSoulmateToken() for users that have a soulmate. However for users without a soulmate, this mapping will return 0 by default.

When the function then calls the Soulmate::idToCreationTimestamp() function to fetch the timestamp of when the soulmates were united, it will actually return the timestamp of the very first soulmate NFT minted OR if no one has minted, this will also return 0.

If we assume a soulmate NFT has been minted, the function Airdrop::claim() will then continue to do calculations based off of the first NFT minted and transfer tokens to the caller msg.sender as if they were the first soulmate NFT minted.

In the case there has been no Soulmate NFTs minted, the Airdrop::claim() function will set Airdrop::numberOfDaysInCouple to the amount of days since block.timestamp's inception: (block.timestamp - 0) / 3600 * 24.

Impact

Any user can be rewarded with tokens without having actually minted a soulmate.

POC

Imagine the following most-likely scenario:

  1. User A calls Soulmate::mintSoulmateToken().

  2. User B calls Soulmate::mintSoulmateToken() and now NFT #0 has been minted.

  3. 10 days passes by since NFT #0 was minted.

  4. User C (Attacker) calls Airdrop::claim().

  5. Airdrop::claim() checks the number of days passed. Since user C hasn't minted, it will set the days since NFT #0 was minted.

  6. Airdrop::claim() rewards user C with 10 tokens.

Here is a POC of the above scenario:

function test_claimWithoutASoulmate() public {
_mintOneTokenForBothSoulmates();
​
vm.warp(block.timestamp + 10 days + 1 seconds);
​
address attacker = makeAddr("attacker");
vm.prank(attacker);
airdropContract.claim();
​
assertTrue(loveToken.balanceOf(attacker) == 10 ether);
}

Tools Used

VS Code, Foundry

Recommendations

Add a new error and make a call to Soulmate::soulmateOf to check if msg.sender has a soulmate, if they don't revert the transaction. This will prevent those who never minted, in addition to those who are still waiting for a soulmate.

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