pkqs90 - `EthosDiscussion#addReply()` does not check if targetContract is an EthosContract.

[sherlock-admin3]

pkqs90

Medium

EthosDiscussion#addReply() does not check if targetContract is an EthosContract.

Summary

EthosDiscussion#addReply does not check if targetContract is an EthosContract. Users can freely pass any address and an reply can be successfully added, polluting the EthosDiscussion database.

Root Cause

In the EthosDiscussion#addReply() function, user can freely pass any targetContract address, as long as it implements the interface function targetExistsAndAllowedForId(uint256 _targetId) external view returns (bool exists, bool allowed); and return true. There is no check to verify if targetContract is an EthosContract or not.

This is an issue because it would pollute the EthosDiscussion database, which is used to serve the frontend Ethos app.

https://github.com/sherlock-audit/2024-10-ethos-network/blob/main/ethos/packages/contracts/contracts/EthosDiscussion.sol#L105

  function addReply(
    address targetContract,
    uint256 targetId,
    string memory content,
    string memory metadata
  ) external onlyNonZeroAddress(targetContract) whenNotPaused {
    uint256 authorID = IEthosProfile(
      contractAddressManager.getContractAddressForName(ETHOS_PROFILE)
    ).verifiedProfileIdForAddress(msg.sender);

    bool isTargetThisContract = _isAddressThisContract(targetContract);

    if (isTargetThisContract) {
      if (replies[targetId].createdAt == 0) {
        revert TargetNotFound(targetContract, targetId);
      }
    } else {
      _checkIfTargetExistsAndAllowed(targetContract, targetId);
    }

    uint256 _replyCount = replyCount;

    directReplyIdsByTargetAddressAndTargetId[targetContract][targetId].push(_replyCount);
    replyIdsByAuthor[authorID].push(_replyCount);

    replies[_replyCount] = Reply(
      !isTargetThisContract,
      targetContract,
      authorID,
      _replyCount,
      targetId,
      block.timestamp,
      0,
      content,
      metadata
    );

    emit ReplyAdded(authorID, targetContract, _replyCount);

    replyCount++;
  }

  function _checkIfTargetExistsAndAllowed(address targetContract, uint256 targetId) private view {
@>  (bool exists, ) = ITargetStatus(targetContract).targetExistsAndAllowedForId(targetId);

    if (!exists) {
      revert TargetNotFound(targetContract, targetId);
    }
  }

In contrast, we can see that EthosVote implemented a isValidTarget() check to make sure the targetContract is always a valid EthosContract.

https://github.com/sherlock-audit/2024-10-ethos-network/blob/main/ethos/packages/contracts/contracts/EthosVote.sol#L141

  modifier isValidTarget(address target) {
    if (!contractAddressManager.checkIsEthosContract(target)) {
      revert InvalidTargetContract(target);
    }
    _;
  }

  function voteFor(
    address targetContract,
    uint256 targetId,
    bool isUpvote
@>) external whenNotPaused isValidTarget(targetContract) {
    (bool exists, ) = ITargetStatus(targetContract).targetExistsAndAllowedForId(targetId);

    if (!exists) {
      revert TargetNotFound(targetContract, targetId);
    }

    uint256 voter = IEthosProfile(contractAddressManager.getContractAddressForName(ETHOS_PROFILE))
      .verifiedProfileIdForAddress(msg.sender);

    if (hasVotedFor(voter, targetContract, targetId)) {
      _modifyVote(targetContract, targetId, voter, isUpvote);
    } else {
      _recordVote(voter, targetContract, targetId, isUpvote);
    }
  }

Internal pre-conditions

N/A

External pre-conditions

N/A

Attack Path

N/A

Impact

Users can pollute the EthosDiscussion by adding replies to a random contract that does not belong in Ethos.

PoC

N/A

Mitigation

Similar to EthosVote, add a contractAddressManager.checkIsEthosContract(target) check.