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

The lack of checks for empty messages and prolonged messages is a significant vulnerability. Could lead to a `DoS` (Denial of Service) issue.

Summary

The Soulmate::writeMessageInSharedSpace function currently lacks checks for both empty messages and prolonged messages. While empty messages are acceptable to ensure some form of communication, it's imperative to implement a check for message length. Longer messages incur a higher GAS and could potentially lead to significant resource consumption. Therefore, implementing a length check is essential to mitigate this risk.

function writeMessageInSharedSpace(string calldata message) external {
// -->| // here should have a check for message maximum length.
uint256 id = ownerToId[msg.sender];
sharedSpace[id] = message;
emit MessageWrittenInSharedSpace(id, message);
}

Vulnerability Details

DoS (GAS Consumption):
  1. Place the following test code snippet into the test/unit/soulmateTest.t.sol file. Put it at the very bottom but before the last closing semicolon }.

function test_denialOfService() public {
address alice = makeAddr("ALICE");
address bob = makeAddr("BOB");
vm.startPrank(alice);
soulmateContract.mintSoulmateToken();
vm.stopPrank();
vm.startPrank(bob);
soulmateContract.mintSoulmateToken();
vm.stopPrank();
vm.txGasPrice(1);
uint256 gasBeforeMsgTransmission = gasleft();
vm.startPrank(bob);
string memory message = "I <3 U";
soulmateContract.writeMessageInSharedSpace(message);
vm.stopPrank();
uint256 gasAfterMsgTransmission = gasleft();
uint256 totalGasCost = (gasBeforeMsgTransmission - gasAfterMsgTransmission) * tx.gasprice;
console2.log("GAS Cost for Sending a message of length 6: ", totalGasCost);
uint256 gasBeforeMsgTransmission_two = gasleft();
vm.startPrank(bob);
string memory newMessage =
"I want to express my love for you. You are my everything; I cannot imagine life without you. You may not realize how lonely I was before our paths crossed";
soulmateContract.writeMessageInSharedSpace(newMessage);
vm.stopPrank();
uint256 gasAfterMsgTransmission_two = gasleft();
uint256 totalGasCost_two = (gasBeforeMsgTransmission_two - gasAfterMsgTransmission_two) * tx.gasprice;
console2.log("GAS Cost for Sending a message of bigger length: ", totalGasCost_two);
uint256 gasBeforeMsgTransmission_three = gasleft();
vm.startPrank(bob);
string memory newMessage_big =
"I want to express my love for you. You are my everything; I cannot imagine life without you. You may not realize how lonely I was before our paths crossed. I want to express my love for you. You are my everything; I cannot imagine life without you. You may not realize how lonely I was before our paths crossed. I want to express my love for you. You are my everything; I cannot imagine life without you. You may not realize how lonely I was before our paths crossed. I want to express my love for you. You are my everything; I cannot imagine life without you. You may not realize how lonely I was before our paths crossed.";
soulmateContract.writeMessageInSharedSpace(newMessage_big);
vm.stopPrank();
uint256 gasAfterMsgTransmission_three = gasleft();
uint256 totalGasCost_three = (gasBeforeMsgTransmission_three - gasAfterMsgTransmission_three) * tx.gasprice;
console2.log("GAS Cost for Sending a message of more bigger length: ", totalGasCost_three);
}
  1. Open Your Bash Terminal and execute the following command...

forge test --mt "test_UnknownWriteSharedSpace" -vv --via-ir
  1. The output should clearly demonstrate that GAS costs increase with message length. This escalation in costs raises concerns about the potential for a Denial of Service (DoS) attack. In such a scenario, a soulmate could exhaust all their ETH reserves simply to cover the GAS costs associated with sending messages.

Impact

As GAS Cost grow with a message length, Sending a message could lead to heavy GAS Consumption and could cause a Denial of Service DoS in the End.

Tools Used

Foundry Framework (Solidity, Rust)

Recommendations

Mitigation is simple, add checks to limit messages length and we can also add a check to avoid empty messages.

Update the src/Soulmate.sol file with the following code modifications...

...
...
...
error Soulmate__alreadyHaveASoulmate(address soulmate);
error Soulmate__SoulboundTokenCannotBeTransfered();
+ error Soulmate__CannotAddEmptyOrMoreThan50CharsLongMessage(uint256 messageLength);
...
...
...
function writeMessageInSharedSpace(string calldata message) external {
+ uint256 messageLength = abi.encodePacked(message).length;
+ if (messageLength > 50 || messageLength == 0) {
+ revert Soulmate__CannotAddEmptyOrMoreThan50CharsLongMessage(messageLength);
+ }
uint256 id = ownerToId[msg.sender];
sharedSpace[id] = message;
emit MessageWrittenInSharedSpace(id, message);
}
...
...
...

After modifying and updating the Soulmate.sol file, try to re-execute the test discussed in the Proof of Concept (PoC). It should get reverted with the error Soulmate__CannotAddEmptyOrMoreThan50CharsLongMessage at some point hopefully.

Updates

Lead Judging Commences

0xnevi Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Other

Support

FAQs

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