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

Lows

heavenz52

First couple may get divorced due to corrupted sharedMessage space

/* Corrupted sharedSpace */
function test_CorruptedSharedSpaceMayCauseDivorce() public {
//Step 1:In the beginning, soulmate1 and soulmate2 become soulmates of each other
vm.prank(soulmate1);
soulmateContract.mintSoulmateToken();
​
vm.prank(soulmate2);
soulmateContract.mintSoulmateToken();
assertTrue(soulmateContract.soulmateOf(soulmate1) == soulmate2);
assertTrue(soulmateContract.soulmateOf(soulmate2) == soulmate1);
​
//Step 2: soulmate1 says "I love you "
vm.prank(soulmate1);
soulmateContract.writeMessageInSharedSpace("I love you");
​
//Step 3: Random person corrupts the message
address randomPerson = makeAddr("randomPerson");
vm.prank(randomPerson);
soulmateContract.writeMessageInSharedSpace("I am not in love with you anymore");
​
//Step 4: soulmate2 reads "I am not in love with you anymore"
vm.prank(soulmate2);
string memory message = soulmateContract.readMessageInSharedSpace();
string[4] memory possibleText = [
"I am not in love with you anymore, sweetheart",
"I am not in love with you anymore, darling",
"I am not in love with you anymore, my dear",
"I am not in love with you anymore, honey"
];
bool found;
for (uint i; i < possibleText.length; i++) {
if (keccak256(abi.encodePacked(possibleText[i])) ==
keccak256(abi.encodePacked(message))) {
found = true;
break;
}
}
assertTrue(found);
​
//Step 5: soulmate2 files for divorce
vm.startPrank(soulmate2);
soulmateContract.getDivorced();
assertTrue(soulmateContract.isDivorced());
vm.stopPrank();
​
}

Note - This happens to the very first couple that is formed because there is no zero check for id made here in Soulmate.sol

Consider reverting with an error in such cases

Selfmate instead of soulmate!

There is no check to see that the two soulmates are different

/*
A person can become his soulmate.
Consequences
- He can then receive a love token and wait to claim airdrop rewards
- totalSouls() would return the wrong value of 2 instead of 1 (as it's the same person)
*/
function test_AnyoneCanBecomeTheirOwnSoulmate() public {
vm.startPrank(soulmate1);
soulmateContract.mintSoulmateToken();
soulmateContract.mintSoulmateToken();
vm.stopPrank();
​
assertTrue(soulmateContract.soulmateOf(soulmate1) == soulmate1);
assertTrue(soulmateContract.totalSouls() == 2);
}

Solo divorce leads to misleading events emitted

A single person who hasn't found his soulmate can divorce himself

Check should be made that it's a couple

/*
A noncouple can also get divorced (Basically, a single person can divorce himself)
*/
function test_SinglePersonCanGetDivorced() public {
address randomPerson = makeAddr("randomPerson");
vm.startPrank(randomPerson);
​
soulmateContract.getDivorced();
assertTrue(soulmateContract.isDivorced());
​
vm.stopPrank();
}

The event CoupleHasDivorced(randomPerson, randomPerson) gets emitted (and it's the same person)

Solmate library recommendations

ERC20 functions like transferFrom and transfer should be replaced with their safe counter parts. This will be useful when users are not EOA and they may be controlled by manager contract that needs to listen to hooks like onERC721Received, etc.

Incorrect variable name

Consider renaming the variable to secondsInDay instead of daysInSecond in Airdrop.sol

uint256 public constant daysInSecond = 3600 * 24;

Unused storage variable

Consider getting rid of this variable in LoveToken.sol because it is doesn't serve any purpose

ISoulmate public immutable soulmateContract;

Romanticism may be spoiled

When the sharedSpace[ownerToId[msg.sender]] is empty, readMessageInSharedSpace() may spoil the little touch of romanticism (as claimed in the comments) if the message is empty. Soulmate.sol

string.concat(
@> sharedSpace[ownerToId[msg.sender]], // will be empty
", ",
@> niceWords[block.timestamp % niceWords.length] // niceWords will be shown but without any nice message
);

Computation can be cached to save gas

In here we compute the value numberOfDaysInCouple * 10 ** loveToken.decimals() twice. So that can be cached

Updates

Lead Judging Commences

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

finding-write-message-nft-0-id

Medium Severity, This has an indirect impact and influence on the possibility of divorce between soulmates owning the first soulmate NFT id0, leading to permanent loss of ability to earn airdrops/staking rewards.

Support

FAQs

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