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

Lack of validations at `Airdrop::claim()` allows attackers to claim lovetokens without a Soulbound NFT.

Turetos

Summary

The Airdrop::claim() function lacks validations to prevent token claiming for users without a Soulbound NFT.
This enables attackers to call the claim function and obtain tokens as if they had the first minted Soulbound NFT.

Additionally, there is a flaw in the use of the Soulmate::isDivorced function, where the airdrop contract address is used
instead of the user's address. A divorced user can still claim a Soulbound NFT.

Vulnerability Details

  • The isDivorced check will always pass since it sends the msg.sender of the contract to the function, which consistently returns false for the
    Airdrop contract address.

  • numberOfDaysInCouple is computed using the timestamp when the NFT was minted, but for the attacker without a minted NFT, it returns the timestamp of the first minted NFT.

function claim() public {
// No LoveToken for people who don't love their soulmates anymore.
@> if (soulmateContract.isDivorced()) revert Airdrop__CoupleIsDivorced();
​
@> //No validation here if the msg.sender has a Soulbound NFT
​
// Calculating since how long soulmates are reunited
@> uint256 numberOfDaysInCouple = (block.timestamp -
soulmateContract.idToCreationTimestamp(
soulmateContract.ownerToId(msg.sender)
)) / daysInSecond;
​
...
...
}

Impact

An attacker lacking a Soulbound NFT can claim rewards, receiving them as if they have the first minted NFT.

Here's a test demonstrating two separate claims with a 10-day gap between each.

function testClaimNoSoulmate() public {
console.log("Balance before: ", loveToken.balanceOf(soulmate1) / 10 ** loveToken.decimals());
​
vm.warp(block.timestamp + 10 days + 1 seconds);
​
vm.prank(soulmate1);
airdropContract.claim();
console.log("Balance after claim 1: ", loveToken.balanceOf(soulmate1) / 10 ** loveToken.decimals());
​
vm.warp(block.timestamp + 20 days + 1 seconds);
​
vm.prank(soulmate1);
airdropContract.claim();
​
console.log("Balance after claim 2: ", loveToken.balanceOf(soulmate1) / 10 ** loveToken.decimals());
}
​

Output

Logs:
Balance before: 0
Balance after claim 1: 10
Balance after claim 2: 30
​

Tools Used

Manual review and Foundry

Recommendations

Revise the Airdrop::claim function to include a check verifying that the caller possesses a minted Soulbound NFT, and start counting from nextID = 1, so an id of 0 will be used as invalid.
Adjust the Soulmate::isDivorced function to accurately validate the msg.sender.

/// @notice Claim tokens. Every person who have a Soulmate NFT token can claim 1 LoveToken per day.
function claim() public {
// No LoveToken for people who don't love their soulmates anymore.
- if (soulmateContract.isDivorced()) revert Airdrop__CoupleIsDivorced();
+ if (soulmateContract.isDivorced(msg.sender)) revert Airdrop__CoupleIsDivorced();
​
++ require(soulmateContract.ownerToId(msg.sender) != 0);
​
// Calculating since how long soulmates are reunited
uint256 numberOfDaysInCouple = (block.timestamp -
soulmateContract.idToCreationTimestamp(
soulmateContract.ownerToId(msg.sender)
)) / daysInSecond;
​
...
...
}

Update the isDivorced function to correctly handle the msg.sender as a parameter.

-- function isDivorced() public view returns (bool) {
-- return divorced[msg.sender];
-- }
++ function isDivorced(address soulmate) public view returns (bool) {
++ return divorced[soulmate];
++ }
Updates

Lead Judging Commences

0xnevi Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-isDivorced-wrong-check

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.