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

Missing validation in the `Soulmate.sol::mintSoulmateToken()` function allows users to become soulmates with themselves

stefanlatinovic

Summary

Missing validation check in the Soulmate.sol::mintSoulmateToken() function will allow users to become soulmates with themselves.

Vulnerability Details

The user can become a soulmate with itself by calling the Soulmate.sol::mintSoulmateToken() function twice in a row.

Soulmate.sol::totalSouls() function returns incorrect results if any user is soulmate with themselves.

Impact

Users are allowed to become soulmates with themselves.

Proof of Concept (PoC)

Add the following test in SoulmateTest.t.sol:

function test_usersCanBecomeSoulmateWithThemselves(address random) public {
// Random address validation
vm.assume(random != address(soulmateContract) && random != address(loveToken) &&
random != address(stakingContract) && random != address(airdropContract) &&
random != address(airdropVault) && random != address(stakingVault) &&
random != address(soulmate1) && random != address(soulmate2)
);
​
uint tokenIdMinted = soulmateContract.totalSupply();
​
vm.startPrank(random);
soulmateContract.mintSoulmateToken();
soulmateContract.mintSoulmateToken();
vm.stopPrank();
​
assertEq(soulmateContract.totalSupply(), tokenIdMinted + 1);
assertEq(soulmateContract.soulmateOf(random), random);
assertEq(soulmateContract.ownerToId(random), tokenIdMinted);
assertEq(soulmateContract.ownerOf(tokenIdMinted), random);
​
assertEq(soulmateContract.totalSouls(), 2); // this should not be correct as there is only one user who is a soulmate to themself
}

Run a test with forge test --mt test_usersCanBecomeSoulmateWithThemselves .

Tools Used

  • Manual review

  • Foundry

Recommendations

The transaction should be reverted if a user attempts to become a soulmate with itself.

Recommended changes to the Soulmate.sol::mintSoulmateToken() function:

/*//////////////////////////////////////////////////////////////
ERRORS
//////////////////////////////////////////////////////////////*/
​
error Soulmate__alreadyHaveASoulmate(address soulmate);
error Soulmate__SoulboundTokenCannotBeTransfered();
+error Soulmate__UserCannotBeSoulmateWithItself();
​
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];
if (soulmate1 == address(0)) {
idToOwners[nextID][0] = msg.sender;
ownerToId[msg.sender] = nextID;
emit SoulmateIsWaiting(msg.sender);
} else if (soulmate2 == address(0)) {
+ if (soulmate1 == msg.sender) {
+ revert Soulmate__UserCannotBeSoulmateWithItself();
+ }
idToOwners[nextID][1] = msg.sender;
// Once 2 soulmates are reunited, the token is minted
ownerToId[msg.sender] = nextID;
soulmateOf[msg.sender] = soulmate1;
soulmateOf[soulmate1] = msg.sender;
idToCreationTimestamp[nextID] = block.timestamp;
​
emit SoulmateAreReunited(soulmate1, soulmate2, nextID);
​
_mint(msg.sender, nextID++);
}
​
return ownerToId[msg.sender];
}

Add the following import and test in SoulmateTest.t.sol:

function test_mintSoulmateTokenRevertsWhenUserIsTryingToBecomeSoulmateWithItself(address random) public {
// Random address validation
vm.assume(random != address(soulmateContract) && random != address(loveToken) &&
random != address(stakingContract) && random != address(airdropContract) &&
random != address(airdropVault) && random != address(stakingVault) &&
random != address(soulmate1) && random != address(soulmate2)
);
​
uint tokenIdMinted = soulmateContract.totalSupply();
​
vm.startPrank(random);
soulmateContract.mintSoulmateToken();
vm.expectRevert(Soulmate.Soulmate__UserCannotBeSoulmateWithItself.selector);
soulmateContract.mintSoulmateToken();
vm.stopPrank();
}

Run a test with forge test --mt test_mintSoulmateTokenRevertsWhenUserIsTryingToBecomeSoulmateWithItself.

Updates

Lead Judging Commences

0xnevi Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-self-soulmate

- Given the native anonymous nature of blockchain in general, this issue cannot be avoided unless an explicit whitelist is implemented. Even then we can only confirm soulmates are distinct individuals via kyc. I believe finding a soulmate is intended to be permisionless. - However, even though sufficient (500_000_000e18 in each vault) tokens are minted to claim staking and airdrop rewards, it would take 500_000_000 / 2 combined weeks for airdrop vault to be drained which is not unreasonable given there are [80+ million existing wallets](https://coinweb.com/trends/how-many-crypto-wallets-are-there/). Given there is no option to mint new love tokens, this would actually ruin the functionality of the protocol of finding soulmates and shift the focus to abusing a sybil attack to farming airdrops instead. Assigning medium severity for now but am open for appeals otherwise, since most if not all issues lack indepth analysis of the issue.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.