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

[H-3] `Soulmate.sol::getDivorced` function does not update `soulmateOf`, `idToOwners`, or `ownerToId` mappings.

Keyword

Description: The Soulmate.sol::getDivorced function marks both parties as divorced, but it does not update soulmateOf, idToOwners, or ownerToId mappings to reflect this change in state. Therefore, even after divorce, the contract still considers the parties as connected soulmates in terms of token ownership and permissions

Impact: Divorced soulmates will still accrue staking rewards and are able to use the Soulmate.sol::writeMessageInSharedSpace and Soulmate.sol::readMessageInSharedSpace functions.

Proof of concept: Add this test into the SoulmateTest.t.sol file.
In order for the test to work, you will need to also change the BaseTest.t.sol::_giveLoveTokenToSoulmates function.

function _giveLoveTokenToSoulmates(uint amount) internal {
- _mintOneTokenForBothSoulmates();
uint numberDays = amount / 1e18;
vm.warp(block.timestamp + (numberDays * 1 days));
​
vm.prank(soulmate1);
airdropContract.claim();
​
vm.prank(soulmate2);
airdropContract.claim();
}

We are doing this because inside the test we mint the NFTs manually before divorcing. The _depositTokenToStake function calls this _giveLoveTokenToSoulmates function, which calls _mintOneTokenForBothSoulmates again, and the test will revert with the Soulmate.sol::Soulmate__alreadyHaveASoulmate error. Essentially it tries to mint NFTs again, and the test will not go through.
By temporarily removing this line we can avoid this error, and you can see that divorced soulmates will still accrue staking rewards.

function testDivorcedSoulmatesGetStakingRewards() public {
uint256 balancePerSoulmates = 5 ether;
uint weekOfStaking = 5;
​
vm.prank(soulmate1);
soulmateContract.mintSoulmateToken();
​
vm.prank(soulmate2);
soulmateContract.mintSoulmateToken();
​
vm.prank(soulmate1);
soulmateContract.getDivorced();
​
vm.prank(soulmate2);
soulmateContract.getDivorced();
​
_depositTokenToStake(balancePerSoulmates);
​
vm.warp(block.timestamp + weekOfStaking * 1 weeks + 1 seconds);
​
vm.prank(soulmate1);
stakingContract.claimRewards();
​
assertTrue(loveToken.balanceOf(soulmate1) == weekOfStaking * balancePerSoulmates);
​
vm.prank(soulmate1);
stakingContract.withdraw(balancePerSoulmates);
}

Recommended mitigation: Ensure that the Soulmate.sol::getDivorced function marks both parties as divorced, and updates soulmateOf, idToOwners, or ownerToId mappings to reflect this change in state.

Updates

Lead Judging Commences

0xnevi Lead Judge almost 2 years ago
Submission Judgement Published
Invalidated
Reason: Design choice
Keyword Submitter
almost 2 years ago
0xnevi Lead Judge
almost 2 years ago
0xnevi Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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