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

`soulmate:: mintSoulmateToken` has logic flaw which allows, user to become his own love (in a relationship with himself)

abhishekthakur

Summary

logic flaw, which allows user to become his/her own love (gf/bf), which defeats the whole purpose of soulmate protocol.

Vulnerability Details

Here is the given soulmate:: mintSoulmateToken

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++);
}

The given function checks if in idToOwners mapping, the first index is address(0), assign msg.sender as first soulmate. When first index is filled with other than zero address, it set second index as soulmate and mint the nft to the user who is at second index.

But if you check the highlighted line, it only checks if 2nd index is zero not the msg.sender if it's same as first index. Which allows same person to become his/her soulmate and mint the nft for himself.

POC

In existing test file SoulmateTest.t.sol add the following test to check this vulnerability

function testSoloGamyAkaBeYourselfSoulmate () public {
vm.startPrank(soulmate1);
soulmateContract.mintSoulmateToken();
soulmateContract.mintSoulmateToken();
assertTrue(soulmateContract.totalSupply() == 1);
if(soulmateContract.soulmateOf(soulmate1) == soulmate1){
console2.log("person is relationship with himeself:", soulmate1);
}
}

when you run command forge test --mt testSoloGamyAkaBeYourselfSoulmate -vv in your terminal, it will show following snippet

[⠢] Compiling...
[⠒] Compiling 1 files with 0.8.23
[⠢] Solc 0.8.23 finished in 1.90s
Running 1 test for test/unit/SoulmateTest.t.sol:SoulmateTest
[PASS] testSoloGamyAkaBeYourselfSoulmate() (gas: 182753)
Logs:
person is relationship with himeself: 0x65629adcc2F9C857Aeb285100Cc00Fb41E78DC2f
​
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.55ms
Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests)

Impact

Fail the whole purpose of soulmate protocol

Tools Used

Manual review

Recommendations

Here is the recommendation that can be used to fix the bug -

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)) {
+ } else if (soulmate2 == address(0) && msg.sender != idToOwners[nextID][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++);
}
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.