EthosReview payment does not check if msg.value is zero if price is zero for native Eth.

Summary

EthosReview payment does not check if msg.value is zero if price is zero for native Eth. This may cause users to pay unrequired Eth when adding reviews.

Root Cause

When creating a review, a payment must be paid from the user. If the paymentToken == address(0), it means using native Eth.

There is a check that requires msg.value == price to make sure the user doesn't underpay or overpay the amount of Eth. However, if the expected price is 0, the check is not executed.

Given that an can set the review price of any paymentToken to any price, there may be a case that user will overpay Eth to add a review.

Assume the current review price for Eth is 0.01 Eth. An example is:

User adds a review and sets Eth as payment token, paying 0.01 Eth.
Admin sets Eth price to 0.

Transaction 1 is initiated before transaction 2, but due to blockchain latency, the transaction 2 gets executed first. This means the user didn't need to actually pay the 0.01 Eth, but because the msg.value == price isn't executed, he would pay anyways.

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

  function setReviewPrice(bool allowed, address paymentToken, uint256 price) external onlyAdmin {
    if (allowed) {
      reviewPrice[paymentToken] = PaymentToken({ allowed: allowed, price: price });
    } else {
      delete reviewPrice[paymentToken];
    }
  }

  function _handlePayment(address paymentToken) private onlyValidPaymentToken(paymentToken) {
    uint256 price = reviewPrice[paymentToken].price;

@>  if (price > 0) {
      if (paymentToken == address(0)) {
@>      if (msg.value != price) {
          revert WrongPaymentAmount(paymentToken, msg.value);
        }
      } else {
        if (msg.value > 0) {
          revert WrongPaymentAmount(address(0), msg.value);
        }

        IERC20(paymentToken).transferFrom(msg.sender, address(this), price);
      }
    }
  }

Internal pre-conditions

Admin sets Eth price from non-zero to zero.
User adds an review and it gets "frontrun" by the admin's transaction.

External pre-conditions

N/A

Attack Path

N/A

Impact

User will lose his Eth for adding review when he didn't need to pay.

PoC

Presented above.

Mitigation

Move the check out of the if-statement.

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

pkqs90 - EthosReview payment does not check if msg.value is zero if price is zero for native Eth. #200