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

Missing validation in the `Soulmate.sol::writeMessageInSharedSpace()` function allows NFT non-holders to write in the shared space reserved for holders of the NFT with id `0`

Summary

Missing validation check in the Soulmate.sol::writeMessageInSharedSpace() function will allow NFT non-holders to write in the shared space reserved for holders of the NFT with id 0

Vulnerability Details

The Soulmate.sol::writeMessageInSharedSpace() is implemented as follows:

function writeMessageInSharedSpace(string calldata message) external {
uint256 id = ownerToId[msg.sender];
sharedSpace[id] = message;
emit MessageWrittenInSharedSpace(id, message);
}

ownerToId[msg.sender] will return 0 if msg.sender does not hold an NFT, resulting in a message being written in the shared space of NFT with ID 0.

Impact

NFT non-holders can write messages in the shared space of soulmates that hold NFT with id 0.

Proof of Concept (PoC)

Add the following test in SoulmateTest.t.sol:

function test_NFTNonHoldersCanWriteInTheSharedSpaceReservedForHoldersOfTokenWithIdZero(address random) public {
vm.assume(random != address(soulmateContract) && random != address(loveToken) &&
random != address(stakingContract) && random != address(airdropContract) &&
random != address(airdropVault) && random != address(stakingVault) &&
random != address(soulmate1) && random != address(soulmate2)
);
_mintOneTokenForBothSoulmates(); // token minted for `soulmate1` and `soulmate2`
vm.prank(random);
soulmateContract.writeMessageInSharedSpace("I don't love you anymore!"); // any account that doesn't hold a Soulmate NFT can write in the shared space reserved for holders of the NFT with ID 0
assertEq(soulmateContract.sharedSpace(0), "I don't love you anymore!");
}

Run a test with forge test --mt test_NFTNonHoldersCanWriteInTheSharedSpaceReservedForHoldersOfTokenWithIdZero.

Tools Used

  • Manual review

  • Foundry

Recommendations

The transaction should be reverted if the user attempts to become a soulmate with itself.

Recommended changes to the Soulmate.sol::mintSoulmateToken() function:

/*//////////////////////////////////////////////////////////////
ERRORS
//////////////////////////////////////////////////////////////*/
error Soulmate__alreadyHaveASoulmate(address soulmate);
error Soulmate__SoulboundTokenCannotBeTransfered();
+error Soulmate__CannotWriteToSharedSpaceOfOtherSoulmates();
function writeMessageInSharedSpace(string calldata message) external {
+ if (soulmateOf[msg.sender] == address(0)) {
+ revert Soulmate__CannotWriteToSharedSpaceOfOtherSoulmates();
+ }
uint256 id = ownerToId[msg.sender];
sharedSpace[id] = message;
emit MessageWrittenInSharedSpace(id, message);
}

Add the following import and test in SoulmateTest.t.sol:

function test_writeMessageInSharedSpaceRevertsWhenDoesntHaveSoulmate(address random) public {
vm.assume(random != address(soulmateContract) && random != address(loveToken) &&
random != address(stakingContract) && random != address(airdropContract) &&
random != address(airdropVault) && random != address(stakingVault) &&
random != address(soulmate1) && random != address(soulmate2)
);
_mintOneTokenForBothSoulmates(); // token minted for `soulmate1` and `soulmate2`
vm.prank(random);
vm.expectRevert(Soulmate.Soulmate__CannotWriteToSharedSpaceOfOtherSoulmates.selector);
soulmateContract.writeMessageInSharedSpace("I don't love you anymore!"); // account that doesn't have a soulmate can't write in the shared space
}

Run a test with forge test --mt test_writeMessageInSharedSpaceRevertsWhenDoesntHaveSoulmate.

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.