First Flight #9: Soulmate

Summary

The Soulmate::readMessageInSharedSpace allows couples to read messages in shared space associated with their token id. The function relies on the ownerToId mapping to determine the id associated with the caller's address. If a caller who does not have a soulmate attempts to read a message, the ownerToId mapping should not have an entry for his address, and it should default to 0 since mappings in Solidity return the default value for the value type when accessed with a key that does not exist.That means that the unauthorized user can read message that is written in shared space with id equals to 0.

Vulnerability Details

The function Soulmate::readMessageInSharedSpace allows the person to read a message from his soulmate. The function is public and there is no check if the caller of the function has actually a soulmate. Therefore, people who don't have a soulmate can also calls the function and read a message in shared space with id equals to 0.

The function Soulmate::readMessageInSharedSpace get the id token of the caller (msg.sender) and returns the message in sharedSpace associated with this id.
If the caller is waiting for a soulmate, he will have a valid id and nobody will write him. Therefore, he can get only message that consists from one of the niceWords. Maybe, this is useless.
But if the caller doesn't have a soulmate and his address doesn't exist in the ownerToId mapping, the return value from the ownerToId[msg.sender] will be 0. So the message will be read from the sharedSpace with id equals to 0. The problem is that the id equals to 0 is reserved for the first couple of the protocol. And the unauthorized user can read the message that is between the soulmates of the couple.
Another problem of the functions Soulmate::readMessageInSharedSpace and Soulmate::writeMessageInSharedSpace is that only the last message is stored and respectively only the last message can be read.

Impact

Let's consider the following scenario. There is a couple who use the protocol (soulmate1 and soulmate2). Their token id is equal to 0. Then Bob who doesn't have a soulmate calls the function Soulmate::readMessageInSharedSpace and reads the last message between the soulmates. This message can contain sensitive information.

The following test shows the possibility of reading message from a person who doesn't have a soulmate in the shared space of the existing couple associated to token id equals to 0. You can add the test in the test file Soulmate.t.sol and execute it with the command: forge test --match-test "test_readMessage".

Also, note that only the last shared message can be read. This leads to confusion if the both soulmates write each other at the same time.

And the data stored on the blockchain can be read by everyone. So it is better to not save sensitive information (like personal messages) on chain.

Tools Used

VS Code, Foundry

Recommendations

Add a check in the Soulmate::readMessageInSharedSpace function that the caller has a soulmate.
One possible way is to check if the id is associated with the address of the caller:

If you run the test function after applying the changes, the test will fail:
[FAIL. Reason: revert: The caller doesn't have soulmate] test_readMessage() (gas: 251883)

If you want the people who are waiting for a soulmate also to not be able to call the function, you can add one more condition in the require statement:

Additionally, it would be better if you add an approach to save the whole chat between soulmates and then to read it, not only the last message.
One more thing to consider: any data on the blockchain is public and everyone can read it. So it would be better if the messages between soulmates are stored off chain. You can store the chat messages off-chain (e.g., on IPFS) and only store a reference (like a hash or URI) on-chain.