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

Misleading Total Souls Count due to Unpaired and Self-Paired Users

paprikrumplikas

Summary

Soulmate::totalSouls inaccurately calculates the total number of paired souls. This function currently returns a value that assumes all minting actions result in successful pairings, thereby doubling the nextID to represent total souls. However, this calculation does not account for users who are still awaiting pairing or those who have been incorrectly paired with themselves, leading to a misrepresented count of active soulmate pairs within the system.

Vulnerability Details

The totalSouls function's simplistic calculation method (return nextID * 2;) overlooks two critical scenarios:

  1. Unpaired Souls: Users who have initiated the minting process but are not yet paired. These users should not contribute to the total count of souls until their pairing is confirmed.

  2. Self-Paired Users: The current logic does not prevent a user from being paired with themselves.

The provided test case illustrates this issue by demonstrating that the totalSouls count can be inaccurate immediately after a minting request and can also reflect an incorrect increment when a user is allowed to pair with themselves.

Proof of code:

function test_numberOfSouls() public {
uint256 totalSouls = soulmateContract.totalSouls();
console2.log(totalSouls);
assertEq(totalSouls, 0);
​
vm.prank(user1);
soulmateContract.mintSoulmateToken();
totalSouls = soulmateContract.totalSouls();
console2.log(totalSouls);
assertLt(totalSouls, 1);
​
vm.prank(user1);
soulmateContract.mintSoulmateToken();
totalSouls = soulmateContract.totalSouls();
console2.log(totalSouls);
assertGt(totalSouls, 1);
}

Impact

The inaccurate reporting of total souls impacts the transparency and reliability of the protocol's metrics. It could mislead users and stakeholders about the platform's activity level and the actual number of successful pairings, potentially affecting user trust and engagement.

Tools Used

Manual review.

Recommendations

The following modifications are recommended:

  1. Prevent Self-Pairing: Implement checks within the minting function to prevent users from being paired with themselves, ensuring that all pairings are between distinct users:

function mintSoulmateToken() public returns (uint256) {
// Check if people already have a soulmate, which means already have a token
address soulmate = soulmateOf[msg.sender];
if (soulmate != address(0)) {
revert Soulmate__alreadyHaveASoulmate(soulmate);
}
address soulmate1 = idToOwners[nextID][0];
address soulmate2 = idToOwners[nextID][1];
if (soulmate1 == address(0)) {
idToOwners[nextID][0] = msg.sender;
ownerToId[msg.sender] = nextID;
emit SoulmateIsWaiting(msg.sender);
} else if (soulmate2 == address(0)) {
+ require(msg.sender != soulmate1, "Can't be your own soulmate!");
idToOwners[nextID][1] = msg.sender;
// Once 2 soulmates are reunited, the token is minted
ownerToId[msg.sender] = nextID;
soulmateOf[msg.sender] = soulmate1;
soulmateOf[soulmate1] = msg.sender;
idToCreationTimestamp[nextID] = block.timestamp;
​
emit SoulmateAreReunited(soulmate1, soulmate2, nextID);
​
_mint(msg.sender, nextID++);
}
​
return ownerToId[msg.sender];
}

  1. Rename the function and change its implementation so that it returns the number of pairs, not the number of souls:

- function totalSouls() external view returns (uint256) {
- return nextID * 2;
+ function totalPairs() external view returns (uint256) {
+ return nextID;
}
Updates

Lead Judging Commences

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

finding-totalSouls-wrong-value

Low severity, given `totalSouls()` is simply a view function not used anywhere else in the protocol. There are several instances that can cause wrong values: 1. When there are pending soulmates not yet paired, but `nextId` has already been incremented 2. Divorced soulmates are still included in computation of totalSouls

Support

FAQs

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