Beginner FriendlyFoundryNFT
100 EXP
View results
Submission Details
Severity: medium
Valid

Lovers Can Be Their Own Soulmates to Avoid Divorces

Summary

It is possible to call the mintSoulmateToken() function of the Soulmate.sol contract twice in a row to be your own soulmate. This would ensure the caller gets the Soulmate NFT and avoids being divorced.

Vulnerability Details

Below is the affected mintSoulmateToken() function. Notice it is incorrectly keeping track of the first and second caller.

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)) {
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];
}

Impact

Attackers could guarantee they will never get divorced by being their own soulmates. This would ensure they get airdrops everyday. This was rated as high risk because it is affecting one of the main features of the project and because it is easy to increase the impact by using multiple accounts.

Proof of Concept

Add the following test to SoulmateTest.t.sol and run it with forge test --mt test_SameMintNewToken -vvvv.

function test_SameMintNewToken() public {
// Call mint for the first time. No minted NFT
vm.prank(soulmate1);
soulmateContract.mintSoulmateToken();
assertEq(soulmateContract.totalSupply(), 0);
// Call mint for the second time. Now should have 1 NFT
vm.prank(soulmate1);
soulmateContract.mintSoulmateToken();
assertEq(soulmateContract.totalSupply(), 1);
// Verify the soulmate of soulmate1 is soulmate1
assertEq(soulmateContract.soulmateOf(soulmate1), address(soulmate1));
assertEq(soulmateContract.balanceOf(soulmate1), 1);
}

Tools Used

Foundry and manual analysis.

Recommendations

Add a check to make sure it is not possible to marry yourself. This could be done by verifying that idToOwners[nextID][0] is different than msg.sender in the highlighted line of the "Vulnerability Details" section above.

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.