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

Any non-minter can claim airdrop rewards by incorrectly identifying himself as the owner of the NFT at index 0

0xlrivo

Summary

Airdrop::claim() allows non-minters to fraudulently claim airdrop rewards by falsely identifying themselves as the owner of the NFT at index 0.

Vulnerability Details

The Soulmate::ownerToId() function incorrectly returns 0 for both the actual owners of the corresponding NFT's index and for individuals who have not yet minted an NFT. Consequently, non-minters can exploit this flaw by falsely asserting ownership of the NFT at index 0, thereby claiming airdrop rewards based on the minting time of that NFT.

Impact

The vulnerability enables unauthorised individuals to illegitimately accumulate rewards over time, thereby increasing the daily number of claimable tokens.

Example: after 200 days all non-minters can potentially claim 200 tokens each

Tools Used

Add the following uint test to AirdropTest.t.sol:

function test_anyoneCanClaimAirdopRewards() public {
_mintOneTokenForBothSoulmates();
​
uint256 passedDays = 200;
vm.warp(block.timestamp + passedDays * 1 days + 1 seconds);
​
vm.prank(attacker);
airdropContract.claim();
​
assertTrue(loveToken.balanceOf(attacker) == passedDays * 1 ether);
}

Recommendations

  1. make Soulmate::nextId start from 1 instead of 0

  2. rename the mapping Soulmate::ownerToId to Soulmate::_ownerToId and make it private

  3. add a wrapper public function to the now private mapping Soulmate::_ownerToId that throws if the returned id is 0

mapping(address owner => uint256 id) private _ownerToId;
​
function ownerToId(address _owner) public view returns (uint256) {
uint256 id = _ownerToId[_owner];
if (id != 0) return id;
else revert Soulmate_invalidId();
}

By renaming you don't have to change all the other contracts that requires Soulmate::ownerToId() to work.

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.