Beginner FriendlyFoundryNFT
100 EXP
View results
Submission Details
Severity: low
Valid

```Soulmate::totalSouls``` inaccurate total souls count after divorce

Summary

The Soulmate::totalSouls function is designed to calculate the total number of "souls" by multiplying the nextID by 2. This method assumes that each increment of nextID represents a pair of soulmates, thus doubling the count. However, the Soulmate::getDivorced function does not adjust the nextID or remove any soulmates from the system upon divorce. It marks only the divorced status of both members of the couple as true.

Vulnerability Details

function getDivorced() public {
address soulmate2 = soulmateOf[msg.sender];
divorced[msg.sender] = true;
divorced[soulmateOf[msg.sender]] = true;
@> emit CoupleHasDivorced(msg.sender, soulmate2);
}
function totalSouls() external view returns (uint256) {
@> return nextID * 2;
}

Impact

The current implementation of Soulmate::totalSouls does not account for the divorce status of couples. As a result, it continues to count all pairs that have been created, including those that have divorced. This leads to an inaccurate representation of the total number of active couples in the system.

function testWrongTotalSoulAfterDivorce() public {
address soulmate3 = makeAddr("soulmate3");
address soulmate4 = makeAddr("soulmate4");
//First soulmate couple
vm.prank(soulmate1);
soulmateContract.mintSoulmateToken();
vm.prank(soulmate2);
soulmateContract.mintSoulmateToken();
//Second soulmate couple
vm.prank(soulmate3);
soulmateContract.mintSoulmateToken();
vm.prank(soulmate4);
soulmateContract.mintSoulmateToken();
assertEq(soulmateContract.totalSouls(), 4);
vm.startPrank(soulmate1);
soulmateContract.getDivorced();
assertEq(soulmateContract.totalSouls(), 2);
}
[FAIL. Reason: assertion failed] testWrongTotalSoulAfterDivorce() (gas: 478541)
Logs:
Error: a == b not satisfied [uint]
Left: 4
Right: 2

The souls are 2 (the other 2 have divorced) but the system continues to count 4.

Tools Used

Manual review

Recommendations

Consider maintaining a separate counter for active couples.

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.