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

`Soulmate::getDivorced` allows a user to get divorced even though they have no soulmate

shikhar229169

Summary

Soulmate::getDivorced is used by soulmates to get divorce but it also allows users with no soulmate to get divorced.

Even though one doesn't have any soulmate still it doesn't revert and make divorced mapping for the msg.sender to true.

Along with that it should revert if the soulmates are divorced but still it again modifes the state to same values unnecessarily.

Vulnerability Details

The vulnerability is present in the Soulmate::getDivorced function where it doesn't revert for user having no soulmate and make the divorced mapping to true for them.

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

Here, if the caller, i.e. msg.sender has no soulmate, then soulmate2 will be equal to address(0) and then it makes:

  • divorced for caller to true (even though they have no soulmate to divorce to)

  • makes divorced for address(0) to true.

Along with that it doesn't revert for already divorced soulmates and unnecessarily updates the state to same values.

Impact

  • Allows a user who has no soulmate to successfully call it get divorced mapping for them to true.

Tools Used

Manual Review

PoC

Add the test in the file: test/unit/SoulmateTest.t.sol

Run the test:

forge test --mt test_UsersHavingNoSoulmateCanAlsoCallForDivorce
function test_UsersHavingNoSoulmateCanAlsoCallForDivorce() public {
// here the user `soulmate1` has no soulmate
assertEq(soulmateContract.soulmateOf(soulmate1), address(0));
​
// soulmate1 calls divorce function
vm.prank(soulmate1);
soulmateContract.getDivorced();
​
// here as the user soulmate1 has no soulmates, then there is no point of divorce
// but still due to not implementing necessary checks, users having no soulmate can also get divorce
vm.prank(soulmate1);
assertEq(soulmateContract.isDivorced(), true);
​
// as soulmate1 has no soulmate, therefore along with them for address(0), the mapping divorced becomes true
vm.prank(address(0));
assertEq(soulmateContract.isDivorced(), true);
}

Recommendations

Revert if user has no soulmate.

+ error Soulmate__CallerHasNoSoulmate();
​
function getDivorced() public {
address soulmate2 = soulmateOf[msg.sender];
+ if (soulmate2 == address(0)) {
+ revert Soulmate__CallerHasNoSoulmate();
+ }
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.