PNS - An incorrectly identified author will not be able to archive or restore reviews

[sherlock-admin2]

PNS

Medium

An incorrectly identified author will not be able to archive or restore reviews

Summary

Archiving and restoring reviews is wrongly authorized by msg.sender instead of by profile id.

Root Cause

The first-class citizen in the project is a profile. Such a profile can be identified by multiple addresses that can act on its behalf. The author of the review is always the profile, and the profile can edit the review.

The problem will appear in the archiveReview and restoreReview functions, where the author is identified by the msg.sender address. Such identification will prevent other addresses authorized to act on behalf of the profile from accessing these functions. The situation can be compared to a company that uses many logins to run one profile. All authorized persons should be able to perform actions, even if one employee has their access revoked.

File: ethos/packages/contracts/contracts/EthosReview.sol
  287:   function archiveReview(uint256 reviewId) external whenNotPaused {

  299: 
  300:     if (review.author != msg.sender) { //audit
  301:       revert UnauthorizedArchiving(reviewId);
  302:     }
  303: 

  307:   }

EthosReview.archiveReview

File: ethos/packages/contracts/contracts/EthosReview.sol
  313:   function restoreReview(uint256 reviewId) external whenNotPaused {
  320: 
  321:     if (review.author != msg.sender) { //audit
  322:       revert UnauthorizedArchiving(reviewId);
  323:     }

  332:   }

EthosReview.restoreReview

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

No response

Impact

A user who connects multiple addresses to a profile, e.g. for security purposes, after losing access to one of the addresses should be able to perform actions using other connected addresses. Identification by address instead of profile id will deprive him of access, making the above-mentioned functions useless.

PoC

No response

Mitigation

As with editing, the author should be identified by ID.

File: ethos/packages/contracts/contracts/EthosReview.sol
  261:   function editReview(
  262:     uint256 reviewId,
  263:     string calldata comment,
  264:     string calldata metadata
  265:   ) external whenNotPaused {

  269:     if (review.authorProfileId != authorProfileId) { //audit:info
  270:       revert UnauthorizedEdit(reviewId);
  271:     }

  281:   }

[sherlock-admin2]

The protocol team fixed this issue in the following PRs/commits: https://github.com/trust-ethos/ethos/pull/1763