Skip to content

Latest commit

 

History

History
113 lines (79 loc) · 4.46 KB

037.md

File metadata and controls

113 lines (79 loc) · 4.46 KB

Quiet Ocean Boar

High

hacked contract can be used to drain user's funds

Summary

The protocol accepts issues where a malicious user hacks (takes over admin functions) a contract to be a valid issue as can be observed from readMe statements

The consequences of these contracts being hacked should never result in users losing money. These contracts explicitly should not ever allow the user to maintain funds (escrow, tokens, or other balances). We consider these "non-financial" smart contracts; they are "social" only.

The problem occurs in EthosReview.sol because a hacked contract does allow for users to lose money.

Root Cause

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

  function addReview(
    Score score,
    address subject,
    address paymentToken,
    string calldata comment,
    string calldata metadata,
    AttestationDetails calldata attestationDetails
  ) external payable whenNotPaused {

The function allows a user to leave a review and if the user's paymentToken is an erc20 he must pay a small price.

The problem occurs because if we explore a scenario where the contract is hacked, if a user has unlimited approval to this contract. When the user goes to call addReview with wETH as his payment token, the malicious user can frontrun his tx and set the price to a very high number thus draining the user who was leaving the review.

  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);
      }
    }
  }

As we can see the logic will transferFrom an amount equal to price from the user.

The malicious user may front run the victims tx and change price right before the tx

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

The user thought he was paying a fair price but the hacker was able to drain the innocent user.

Additionally even if the user does not set infinite approval the attack can still work if the user submits 2 reviews in quick succession. The user will have to approve enough for those 2 transactions, the malicious user can frontrun both transaction and set the price to double to force the user to pay double the price for 1 review.

Although in sherlock admin protected actions are deemed invalid, due to the readMe comments, the issue should be marked as valid high

From readMe

The consequences of these contracts being hacked should never result in users losing money. These contracts explicitly should not ever allow the user to maintain funds (escrow, tokens, or other balances). We consider these "non-financial" smart contracts; they are "social" only.

from Sherlock Docs:

Hierarchy of truth: If the protocol team provides no specific information, the default rules apply (judging guidelines).

the protocol does provide specific information so the default rules will not apply

also from sherlock docs:

If the protocol team provides specific information in the README or CODE COMMENTS, that information stands above all judging rules. In case of contradictions between the README and CODE COMMENTS, the README is the chosen source of truth.

Internal pre-conditions

  1. user must use an erc20 as payment token
  2. contract must be hacked(a malicious user controls an admin account)

External pre-conditions

none

Attack Path

  1. contract is hacked(malicious user controls an admin account)
  2. user sees that price is 5 (using 5 for simplicity)
  3. user approves a large value to the contract(can also work if he does not approve large value as described above)
  4. user calls addReview
  5. malicious user frontruns his transaction
  6. the user is drained
  7. malicious user can withdraw funds

Impact

theft of funds/ loss of funds

PoC

No response

Mitigation

allow a user to specify the price he is paying to avoid any frontrunning issue.