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

Anyone Can Claim the Airdrop of Relationship 0 and drain the Airdrop Contract

EloiManuel

Summary

It is possible for anyone to claim the airdrop rights incurred by the first couple of soulmates (id 0) by just calling the claim() function of the contract Airdrop.sol.

Vulnerability Details

The first two sections highlighted below, and in the "Relevant GitHub Links", show how the claim() function is making wrong assumptions about the returned values by the soulmateContract contract.

First, it is assuming that if the caller is not divorced, it does have a soulmate. However, it could be the case where the caller does not have a soulmate hence the reason why isDivorced() would return false.

Then, the function is calculating the number of days that the couple has been together to determine the airdrop amount. For that, it populates the local variable numberOfDaysInCouple with soulmateContract.idToCreationTimestamp(soulmateContract.ownerToId(msg.sender)). Notice that soulmateContract.ownerToId(msg.sender) would always return 0 for addresses that do not have a soulmate. Therefore, the numberOfDaysInCouple local variable would be equal to soulmateContract.idToCreationTimestamp(0), so the creation timestamp of the first couple (id 0).

Finally, the flaw can be exploited by multiple addresses until the contract is drained because of the third highlighted part of the code, where the _claimedBy mapping is used. This allows different accounts to exploit the vulnerability explained above several times as _claimedBy will always be 0 for new addresses.

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

Impact

An attacker could call the claim() function of the Airdrop.sol contract multiple times using different accounts until it is completely empty.

Proof of Concept

Add the following test to AirdropTest.t.sol and run it with forge test --mt test_ClaimByAttackers -vvvv. The test is calling claim() 10 times using different accounts.

function test_ClaimByAttackers() public {
// Pass 200 days to generate right to claim the airdrop
vm.warp(block.timestamp + 200 days + 1 seconds);
​
for(uint256 i = 1; i <= 10; i++) {
address attacker = vm.addr(i);
vm.prank(attacker);
airdropContract.claim();
assertEq(loveToken.balanceOf(attacker), 200 ether);
}
}

Tools Used

Foundry and manual analysis.

Recommendations

It is recommended to implement a proper mechanism to verify that a couple exists (and it is not divorced), that the account claiming the airdrop does indeed have such rights, and that the same period cannot be claimed several times.

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.