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

Unpaired users can get divorced

paprikrumplikas

Summary

The getDivorced function is supposed to be used by users who have already been paired up with another user, to initiate a divorce between the two. However, a flaw in Soulmate.sol allows users to invoke the getDivorced function regardless of their current pairing status. This includes users who have never been paired (minted a soulmate token) and users who have initiated the minting process but have not yet been paired with another user. The issue allows for a divorce state to be set for users outside the intended logic of requiring a pair to be formed first.

Vulnerability Details

The Soulmate::getDivorced function does not check whether a user is currently in a paired state before allowing the divorce action to proceed. This results in the possibility for a user to:

  • Become divorced without ever having a soulmate.

  • Become divorced while in a waiting state for a soulmate.

  • Impact the logical flow of the contract by allowing users in unintended states to become divorced.

The contract uses a boolean mapping to track divorced states, and the absence of checks allows any user to set their divorced state to true.

Place the following piece of code to SoulmateTest.t.sol:

function test_singleSoulsCanGetDivorced() public {
// user who never called mint can get divorced
// user1 divorced
vm.startPrank(user1);
soulmateContract.getDivorced();
bool isUser1Divorced = soulmateContract.isDivorced();
vm.stopPrank();
assertEq(isUser1Divorced, true); // but user1 can still call mintSoulmateToken and paired up with somebody
​
// user who called mint can get divorced before having been paired up with somebody
// user2 divorced
vm.startPrank(user2);
soulmateContract.mintSoulmateToken();
soulmateContract.getDivorced();
bool isUser2Divorced = soulmateContract.isDivorced();
vm.stopPrank();
assertEq(isUser2Divorced, true); // but user2 remains in waiting state
console2.log("User 2 is divorced: ", isUser2Divorced);
​
// a user can get paired up with someone else who accidentally got divorced as a single
// divorced user2 is paired up with non-divorced user3
vm.prank(user3);
soulmateContract.mintSoulmateToken();
assertEq(user2, soulmateContract.soulmateOf(user3));
assertEq(user3, soulmateContract.soulmateOf(user2));
​
vm.prank(user3);
bool isUser3Divorced = soulmateContract.isDivorced();
assertEq(isUser3Divorced, false);
vm.prank(user2);
isUser2Divorced = soulmateContract.isDivorced();
console2.log("User 3 is divorced: ", isUser3Divorced);
console2.log("User 2 is divorced: ", isUser2Divorced);
assertEq(isUser2Divorced, true);
​
// non-divorced user3 wants to get divorced from divorced user2
vm.startPrank(user3);
soulmateContract.getDivorced();
isUser3Divorced = soulmateContract.isDivorced();
vm.stopPrank();
vm.prank(user2);
isUser2Divorced = soulmateContract.isDivorced();
console2.log("User 3 is divorced: ", isUser3Divorced);
console2.log("User 2 is divorced: ", isUser2Divorced);
assertEq(isUser2Divorced, true);
assertEq(isUser3Divorced, true);
}

Impact

This vulnerability disrupts the intended logic and flow of the Soulmate contract by allowing users to reach a divorced state without ever entering a proper pairing. This could lead to inconsistencies within the contract state, affecting the overall integrity of the system.
For example, consider the following scenario:

  1. UserA calls Soulmate::mintSoulmateToken, and waits to be paired up with somebody else.

  2. Before having been paired up with another user, UserA accidentally calls Soulmate::getDivorced which sets its divorced state to true.

  3. UserB calls Soulmate::mintSoulmateToken and, consequently, is paired up with UserA. Now we have a couple made up by UserA who has a divorced state true, andUserB who has a divorced state false.

  4. Even thoughUserA has not divorced UserB, UserA will not be able to collect its share of love tokens by calling Airdrop::claim due to its divorced flag being true.

Tools Used

Manual review.

Recommendations

Implement checks within getDivorced to ensure that a user is in a valid state to request a divorce:

function getDivorced() public {
+ address soulmate = soulmateOf[msg.sender];
+ require(soulmate != address(0), "Soulmate: Not currently paired");
+ // Further check to ensure the caller is not already divorced
+ require(!isDivorced[msg.sender], "Soulmate: Already divorced");
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.