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

Airdrop vault can be drained by claims from non-Soulmate EOAs mapping to soulmateId of 0

dougo

Summary

In the scenario that the initial Soulmate NFT (soulmateId = 0) has been minted and enough time has passed such that an airdrop can be claimed, multiple non-Soulmate EOAs can call Airdrop::claim() and receive a loveTokens airdrop. This is due to references to soulmateContract.ownerToId(msg.sender) returning the value of 0 for the soulmateId, which corresponds to the very first soulmate NFT which was issued. This allows many non-Soulmate EOAs to claim the same airdrop amount for that NFT over and over until the Airdrop vault is completely drained.

Vulnerability Details

The Airdrop::claim() function relies on soulmateContract.ownerToId(msg.sender) to determine which Soulmate NFT the caller owns. If Airdrop::claim() is called by an address which isn't in the soulmateContract::ownerToId mapping, the value 0 is returned which corresponds to the first minted Soulmate NFT.

The Soulmate::isDivorced() check at the start of Airdrop::claim() also passes due to the calling address not being in the Soulmate::divorced mapping.

As a result, the non-Soulmate caller is considered as a valid Soulmate NFT owner and can receive the airdrop intended for one of the actual owners of the Soulmate NFT (soulmateId = 0). Even worse, multiple non-Soulmate callers can also call Airdrop::claim() and each will receive that airdrop as well, to the point where the Airdrop vault is completely drained.

Impact

High - Although it would take a significant amount of gas to execute the attack, depending upon the attacker's motivation and the market value of the loveToken relative to ETH, the impact of draining the airdrop vault would be catastrophic to the protocol and Soulmate NFT holders who would not be able to claim loveToken airdrops until the vault is replenished. Additionally, the longer the airdrop goes without being claimed the less expensive the attack becomes.

Proof of Concept

The Foundry test below shows how an attacker would drain the airdrop vault.

Add to import at top of Foundry test file

import {Strings} from "@openzeppelin/contracts/utils/Strings.sol";

Add this test to Foundry test file

function testAudit_DrainViaAirdropClaim() public {
// mint tokens for soulmate1 & soulmate 2 using existing protocol test function
_mintOneTokenForBothSoulmates();
​
// move time forward so claim can be made
vm.warp(block.timestamp + 10 days + 1 seconds);
​
// iterate through different non-Soulmate EOAs calling the claim function
address attacker;
uint256 i = 1;
while (loveToken.balanceOf(address(airdropVault)) > 0) {
// generate new attacker address
string memory addressString = string.concat(
"attacker",
Strings.toString(i)
);
attacker = makeAddr(addressString);
​
// prank as new attacker & claim
vm.startPrank(attacker);
airdropContract.claim();
vm.stopPrank();
​
// bump counter so new attacker address gets generated
i++;
}
​
// ideally, airdropVault isn't drained, if it is then fail the test with revert
require(
loveToken.balanceOf(address(airdropVault)) != 0,
"airdrop vault drained!!!"
);
}

When this test is executed, it will fail due to reverting at the end when the airdrop vault's loveToken balance is 0.

Tools Used

Visual Studio Code, Foundry

Recommendations

It is recommended to add a check to the Airdrop::claim() function which validates the caller is a legitimate Soulmate owner.
This can be done by checking Soulmate::soulmateOf() for the caller and requiring that it not return address(0), which is what would be returned when an address is not located in the Soulmate::soulmateOf mapping. For a legitimate Soulmate owner, this mapping would return a non-zero address.

Add a new custom error to the Errors section of the Airdrop contract:

error Airdrop_InvalidCaller();

Next, add a new check to the Airdrop::claim() function:

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_InvalidCaller();
​
/* remainder of function */
}

The test included in Proof of Concept section should now pass.

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.