sherlock-audit / 2024-10-ethos-network-judging

0 stars 0 forks source link

Mahi_Vasisth - Missing _doesReplyExist Check in repliesByAuthorInRange and directRepliesInRange Functions #191

Open sherlock-admin3 opened 2 weeks ago

sherlock-admin3 commented 2 weeks ago

Mahi_Vasisth

Medium

Missing _doesReplyExist Check in repliesByAuthorInRange and directRepliesInRange Functions

Summary:

The functions repliesByAuthorInRange and directRepliesInRange do not include a check for _doesReplyExist before accessing replies by their IDs. This omission can result in accessing non-existent replies, potentially leading to unexpected behavior or errors within the functions.

Vulnerability Details:

The _doesReplyExist function is designed to check if a replyId exists by verifying whether the createdAt timestamp is set. In repliesByAuthorInRange and directRepliesInRange, the absence of this check means that if a replyId does not exist, the functions may proceed without reversion, leading to incorrect or incomplete data in the returned array. This can cause inconsistencies in the replies displayed to users and fail to notify them if they attempt to access non-existent replies.

Code Snippet

https://github.com/sherlock-audit/2024-10-ethos-network/blob/db37b9dc2b792e245eb683d8a956bcb7ef2f1a27/ethos/packages/contracts/contracts/EthosDiscussion.sol#L207-L225

function repliesByAuthorInRange(
    uint256 author,
    uint256 fromIdx,
    uint256 maxLength
  ) external view returns (Reply[] memory result) {
    uint256[] memory replyIds = replyIdsByAuthor[author];
    uint256 length = _correctLength(replyIds.length, maxLength, fromIdx);

    if (length == 0) {
      return result;
    }

    result = new Reply[](length);
    // @audit - _doesReplyExist is missing 
    for (uint256 i = 0; i < length; ++i) {
      uint256 replyId = replyIds[fromIdx + i];
      result[i] = replies[replyId];
    }
  }

https://github.com/sherlock-audit/2024-10-ethos-network/blob/db37b9dc2b792e245eb683d8a956bcb7ef2f1a27/ethos/packages/contracts/contracts/EthosDiscussion.sol#L235-L254

function directRepliesInRange(
    address targetContract,
    uint256 parentId,
    uint256 fromIdx,
    uint256 maxLength
  ) external view returns (Reply[] memory result) {
    uint256[] memory replyIds = directReplyIdsByTargetAddressAndTargetId[targetContract][parentId];
    uint256 length = _correctLength(replyIds.length, maxLength, fromIdx);

    if (length == 0) {
      return result;
    }

    result = new Reply[](length);
    // @audit - _doesReplyExist is missing 
    for (uint256 i = 0; i < length; ++i) {
      uint256 replyId = replyIds[fromIdx + i];
      result[i] = replies[replyId];
    }
  }

Impact:

Without the _doesReplyExist check, users may receive a response that includes non-existent replies or empty data, which can be misleading and may result in unnecessary confusion. Additionally, by failing to revert when a reply is not found, the functions do not follow best practices for error handling, which can lead to unpredictable outcomes in the front end or other parts of the application relying on these functions.

Recommendation:

Add the _doesReplyExist check within the loops in both repliesByAuthorInRange and directRepliesInRange functions. This will ensure that any attempt to access a non-existent reply will revert, providing clear feedback to users about missing replies.