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

A user that mints soul token and is not matched can claim large amound of love tokens in Airdrop contract

kiqo

Summary

If a user calls Soulmate::mintSoulmateToken and is not matched by anyone, they can call Airdrop::claim and receive love tokens proportional to the current block.timestamp.

Vulnerability Details

When a user calls Soulmate::mintSoulmateToken and is not matched the ownerToId mapping is initialized, however the idToCreationTimestampis not. Therefore if a user when minting soulmate token is not matched and calls Airdrop::claim, the calculation of the number of days in couple given by

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

Will result in

uint256 numberOfDaysInCouple = (block.timestamp - 0) / daysInSecond;

Leading to the user being able to remove a great amount of Love Tokens.

POC

Add the following test to BaseTest.t.sol and run forge test --mt test_user_can_drain_funds.

function test_user_can_drain_funds() public {
_mintOneTokenForBothSoulmates();
vm.warp(1707473800); //set a realistic timestamp, 9th February 2024
address soulmate3 = makeAddr("soulmate3");
vm.prank(soulmate3);
soulmateContract.mintSoulmateToken();
​
vm.prank(soulmate3);
airdropContract.claim();
assertEq(loveToken.balanceOf(soulmate3), 19762000000000000000000);
}

Impact

Anyone that mints a soulmate token without being immediately matched will be able to take great amounts of love tokens from the airdrop contract.

Tools Used

Foundry

Recommendations

To address this issue add a check to the airdrop contract that the second soulmate token has already been minted.

In ISoulmate.sol

function ownerToId(address owner) external returns (uint256);
​
+ function idToOwners(uint256 id) external view returns (address[2] memory);
​
function mintSoulmateToken() external returns (uint256);

In Airdrop.sol

​
/*//////////////////////////////////////////////////////////////
ERRORS
//////////////////////////////////////////////////////////////*/
error Airdrop__CoupleIsDivorced();
error Airdrop__PreviousTokenAlreadyClaimed();
+ error Airdrop__SoulmateNotFound();
​
...
​
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 tokenId = soulmateContract.ownerToId(msg.sender);
+ address[2] memory owners = soulmateContract.idToOwners(tokenId);
+ if (owners[1] == address(0)) {
+ revert Airdrop__SoulmateNotFound();
+ }
+ uint256 numberOfDaysInCouple =
+ (block.timestamp - soulmateContract.idToCreationTimestamp(tokenId)) / daysInSecond;
...
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.