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

`Soulmate::writeMessageInSharedSpace` and `Soulmate::readMessageInSharedSpace` allow anyone to read slot linked to id = 0

Summary

Soulmate::writeMessageInSharedSpace and Soulmate::readMessageInSharedSpace allow anyone to read slot linked to id = 0

Vulnerability Details

Soulmate::writeMessageInSharedSpace and Soulmate::readMessageInSharedSpace are functions that allows soulmates to write and read a message inserted in a "private" (Anything written in blockchain is visible) slot of the mapping Soulmate::sharedSpace

function writeMessageInSharedSpace(string calldata message) external {
@> uint256 id = ownerToId[msg.sender];
sharedSpace[id] = message;
emit MessageWrittenInSharedSpace(id, message);
}
/// @notice Allows any soulmates with the same NFT ID to read in a shared space on blockchain.
function readMessageInSharedSpace() external view returns (string memory) {
// Add a little touch of romantism
return
string.concat(
@> sharedSpace[ownerToId[msg.sender]],
", ",
niceWords[block.timestamp % niceWords.length]
);
}

However, because default value in mapping Soulmate::ownerToId is 0, anyone who did not call a function that changes its value in Soulmate::ownerToId will be able to read and write in the slot of ID = 0!

Impact

Alters the functionality of the contract for the first couple to get linked

Tools Used

Foundry

Proof of Concept:
1-soulmate1 and soulmate2 call Soulmate::mintSoulmateToken and get linked, minting the NFT with Id 0. Then soulmate1 calls Soulmate::writeMessageInSharedSpace writing a message in the slot reserved in Soulmate::SharedSpacefor him and his soulmate (Id = 0).
2- soulmate3 calls Soulmate::readMessageInSharedSpace and because he never called any function that changes it default value at Soulmate::ownerToId, it reads the slot for Id =0. Then proceeds to call Soulmate::writeMessageInSharedSpace writing a message in slot 0 because of the same reason as before.

Code
function testReadingId0() public {
address soulmate1 = makeAddr("soulmate1");
address soulmate2 = makeAddr("soulmate2");
address soulmate3 = makeAddr("soulmate3");
vm.prank(soulmate1);
soulmateContract.mintSoulmateToken();
vm.prank(soulmate2);
soulmateContract.mintSoulmateToken();
vm.prank(soulmate1);
soulmateContract.writeMessageInSharedSpace(
"Only my soulmate can read!"
);
//only soulmate 1 and 2 should read this slot!
vm.prank(soulmate3);
string memory message = soulmateContract.readMessageInSharedSpace();
//Prints "Only my soulmate can read!" , the message written by soulmate1! Vulnerability!
console2.log(message);
vm.prank(soulmate3);
soulmateContract.writeMessageInSharedSpace(
"False! any person who didnt call mintSoulmateToken can read and write the slot of Id=0! "
);
vm.prank(soulmate1);
string memory messageRead = soulmateContract.readMessageInSharedSpace();
//Prints "False! any person who didnt call mintSoulmateToken can read and write the slot of Id=0! "
console2.log(messageRead);
//Therefore soulmate3 who didnt even call mintSoulmateToken can indeed write and read in slot id = 0!
}

Recommendations

Adding a check that prevents users without soulmates call that function.

.
.
.
error Soulmate__alreadyHaveASoulmate(address soulmate);
error Soulmate__SoulboundTokenCannotBeTransfered();
+ error Soulmate__senderDoesNotHaveASoulmateYet();
.
.
.
function writeMessageInSharedSpace(string calldata message) external {
+ if (soulmateOf[msg.sender] == address(0)) {revert Soulmate__senderDoesNotHaveASoulmateYet();}
uint256 id = ownerToId[msg.sender];
sharedSpace[id] = message;
emit MessageWrittenInSharedSpace(id, message);
}
/// @notice Allows any soulmates with the same NFT ID to read in a shared space on blockchain.
function readMessageInSharedSpace() external view returns (string memory) {
+ if (soulmateOf[msg.sender] == address(0)) {revert Soulmate__senderDoesNotHaveASoulmateYet();}
// Add a little touch of romantism
return
string.concat(
sharedSpace[ownerToId[msg.sender]],
", ",
niceWords[block.timestamp % niceWords.length]
);
}
Updates

Lead Judging Commences

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

finding-write-message-nft-0-id

Medium Severity, This has an indirect impact and influence on the possibility of divorce between soulmates owning the first soulmate NFT id0, leading to permanent loss of ability to earn airdrops/staking rewards.

Support

FAQs

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