Summary
People can choose themselves as their own Soulmate.
Vulnerability Details
function mintSoulmateToken() public returns (uint256) {
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(soulmate1!=msg.sender);
idToOwners[nextID][1] = msg.sender;
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];
}
After a person successfully sets soulmate1, they can immediately call again to set themselves as soulmate2.
The reason for this is a lack of checks
require(soulmate1!=msg.sender);
This would result in an inaccurate calculation of totalSouls().
function totalSouls() external view returns (uint256) {
return nextID * 2;
}
Impact
People can choose themselves as their own Soulmate.
POC
function test_MintNewToken() public {
uint tokenIdMinted = 0;
vm.startPrank(soulmate1);
soulmateContract.mintSoulmateToken();
soulmateContract.mintSoulmateToken();
assertTrue(soulmateContract.totalSupply() == 1);
assertTrue(soulmateContract.soulmateOf(soulmate1) == soulmate1);
assertTrue(soulmateContract.ownerToId(soulmate1) == tokenIdMinted);
vm.stopPrank();
}
result
[PASS] test_MintNewToken() (gas: 180912)
Traces:
[180912] SoulmateTest::test_MintNewToken()
├─ [0] VM::startPrank(soulmate1: [0x65629adcc2F9C857Aeb285100Cc00Fb41E78DC2f])
│ └─ ← ()
├─ [33015] Soulmate::mintSoulmateToken()
│ ├─ emit SoulmateIsWaiting(soulmate: soulmate1: [0x65629adcc2F9C857Aeb285100Cc00Fb41E78DC2f])
│ └─ ← 0
├─ [133343] Soulmate::mintSoulmateToken()
│ ├─ emit SoulmateAreReunited(soulmate1: soulmate1: [0x65629adcc2F9C857Aeb285100Cc00Fb41E78DC2f], soulmate2: 0x0000000000000000000000000000000000000000, tokenId: 0)
│ ├─ emit Transfer(from: 0x0000000000000000000000000000000000000000, to: soulmate1: [0x65629adcc2F9C857Aeb285100Cc00Fb41E78DC2f], id: 0)
│ └─ ← 0
├─ [393] Soulmate::totalSupply() [staticcall]
│ └─ ← 1
├─ [624] Soulmate::soulmateOf(soulmate1: [0x65629adcc2F9C857Aeb285100Cc00Fb41E78DC2f]) [staticcall]
│ └─ ← soulmate1: [0x65629adcc2F9C857Aeb285100Cc00Fb41E78DC2f]
├─ [607] Soulmate::ownerToId(soulmate1: [0x65629adcc2F9C857Aeb285100Cc00Fb41E78DC2f]) [staticcall]
│ └─ ← 0
├─ [0] VM::stopPrank()
│ └─ ← ()
└─ ← ()
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.76ms
Tools Used
Manual Review
Recommendations
else if (soulmate2 == address(0)) {
require(soulmate1!=msg.sender);
idToOwners[nextID][1] = msg.sender;
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++);
}