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

Missing Access Control, anyone can claim love tokens <3

Aizen

Summary

Missing Access Control in Airdrop::claim() function allows anyone to claim love tokens even though they dont have soulmates.

Vulnerability Details

The Airdrop::claim() function has two checks:

  1. Checks if a soulmate is divorced

  2. Checks if amountAlreadyClaimed >= numberOfDaysInCouple * 10 ** loveToken.decimals()

Now the isDivorced() only checks if a soulmate called the getDivorced() function and broke up with their soulmate.

Code
​
function getDivorced() public {
address soulmate2 = soulmateOf[msg.sender];
divorced[msg.sender] = true;
divorced[soulmateOf[msg.sender]] = true;
emit CoupleHasDivorced(msg.sender, soulmate2);
}
​
function isDivorced() public view returns (bool) {
return divorced[msg.sender];
}

Airdrop::claim() function is ONLY checking if the soulmate is divorced and is not checking if a user even had a soulmate in the first place. Therefore a random user who has never had a soulmate could easily call Airdrop::claim() and get his share of love tokens for the day.

He could also do a block.timestamp manipulation -> Travel to the future 🏃‍♂️💨 -> and claim a HUGE amount of love tokens 🪙

Impact

Random users can claim love tokens without even finding a soulmate. Malicious attackers can manipulate the block.timestamp and empty the vault.

Proof of Code

Add the following code to the AirdropTest.t.sol testsuite

PoC
address evilAlice = makeAddr("bad person");
​
function testAnyoneCanClaimLoveToken() public {
//this is so that it doesn't revert with `Airdrop__PreviousTokenAlreadyClaimed()]`
vm.warp(block.timestamp + 1 days);
​
//evilAlice claims the token without a soulmate
vm.prank(evilAlice);
airdropContract.claim();
}

Tools Used

Manual review

Recommendations

  1. Making custom modifiers to check whether a user is a soulmate of someone and implementing it in the Airdrop::claim() function.

  2. Or adding require statement that checks if a user has a soulmate inside the Airdrop::claim() function.

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.