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

Missing check of existence of soulmate in `Soulmate::getDivorced`

carlosalbawork

Summary

Soulmate::getDivorced does not check if you have a soulmate, therefore allowing for a pair to be one registered as Divorced and the other as Not divorced.

Vulnerability Details

In Soulmate contract there is a function getDivorced that sets to false the value paired with your address in divorced.

function getDivorced() public {
address soulmate2 = soulmateOf[msg.sender];
divorced[msg.sender] = true;
divorced[soulmateOf[msg.sender]] = true;
emit CoupleHasDivorced(msg.sender, soulmate2);
}

However, as you can see, there is no if clause to check if msg.sender is actually linked to a soulmate!

Impact

It allows to divorce before getting a soulmate. Moreover, after being linked with a soulmate, in the divorced mapping, msg.sender appears as divorced (value is true), whereas the soulmate appears as not divorced (value is false) creating a state not expected to be possible by the contract.

I assessed the severity of this finding as Medium due to the lack of consequences it has in the current version of the contract (Does not generate any advantage to the pair or creates malfunction).

Tools Used

Foundry

Recommendations

Proof of Concept:
1- soulmate3 calls Soulmate::mintSoulmateToken when nobody is looking for a soul
2- soulmate3 calls Soulmate::getDivorced before any other user calls Soulmate::mintSoulmateToken, getting its value in divorced changed to true
3- A soulmate4 calls Soulmate::mintSoulmateToken , and gets paired with soulmate3. Neither soulmate3 nor soulmate4 called Soulmate::getDivorced after being paired, so the value of soulmate4 in divorced is false

Code

Add the following code to the SoulmateTest.t.sol file.

​
function testMixDivorcedCouple() public {
address soulmate3 = makeAddr("soulmate3");
address soulmate4 = makeAddr("soulmate4");
​
// Soulmate3 calls when NOBODY is looking for soulmate
vm.startPrank(soulmate3);
soulmateContract.mintSoulmateToken();
soulmateContract.getDivorced();
vm.stopPrank();
​
// Soulmate4 sees that Soulmate3 is looking for soulmate and wants to join him!
vm.startPrank(soulmate4);
soulmateContract.mintSoulmateToken();
vm.stopPrank();
​
vm.prank(soulmate3);
bool divorced3 = soulmateContract.isDivorced();
vm.prank(soulmate4);
bool divorced4 = soulmateContract.isDivorced();
​
//prints true
console2.log(divorced3);
​
//prints false
console2.log(divorced4);
}
​

Recommended Mitigation:

Adding a check to revert if msg.sender does not have a soulmate yet, and a custom error to give verbosity to the situation:

​
.
.
.
error Soulmate__alreadyHaveASoulmate(address soulmate);
error Soulmate__SoulboundTokenCannotBeTransfered();
+ error Soulmate__senderDoesNotHaveASoulmateYet();
.
.
.
​
function getDivorced() public {
+ if(soulmateOf[msg.sender]==address(0)){
+ revert Soulmate__senderDoesNotHaveASoulmateYet() ;
+ }
address soulmate2 = soulmateOf[msg.sender];
divorced[msg.sender] = true;
divorced[soulmateOf[msg.sender]] = true;
emit CoupleHasDivorced(msg.sender, soulmate2);
}
​
Updates

Lead Judging Commences

0xnevi Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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