Skip to content

Latest commit

 

History

History
269 lines (194 loc) · 9.53 KB

032.md

File metadata and controls

269 lines (194 loc) · 9.53 KB

Glamorous Canvas Camel

Medium

Incorrect Cost Calculation in sellVotes Allows Withdrawal of Excess Funds

Summary

The ReputationMarket contract contains an error in the cost calculation within the sellVotes function. Specifically, when calculating the proceeds from selling votes, the contract incorrectly handles negative cost differences by taking the absolute value, which can lead to users withdrawing more funds than they should receive. This miscalculation occurs because the _calcCost function ignores the sign of the cost difference (costDiff), leading to proceeds being incorrectly treated as positive even when they should be negative.

Root Cause

In the _calcCost function, the cost difference (costRatio) calculated by the LMSR.getCost function can be negative when selling votes. However, _calcCost takes the absolute value of costRatio without considering whether it's a gain or a loss, resulting in incorrect cost calculations.

https://github.com/sherlock-audit/2024-12-ethos-update/blob/c3a2b007d0ddfcb476f300f8b766808f0e3e2dfd/ethos/packages/contracts/contracts/ReputationMarket.sol#L1044C1-L1058C7

int256 costRatio = LMSR.getCost(
    market.votes[TRUST],
    market.votes[DISTRUST],
    voteDelta[0],
    voteDelta[1],
    market.liquidityParameter
);

uint256 positiveCostRatio = costRatio > 0 ? uint256(costRatio) : uint256(-costRatio);

// Multiply cost ratio by base price to get cost
cost = positiveCostRatio.mulDiv(
    market.basePrice,
    1e18,
    isPositive ? Math.Rounding.Floor : Math.Rounding.Ceil
);

In the sellVotes function, the proceeds before fees (proceedsBeforeFees) are calculated using _calcCost, which, due to the absolute value handling, returns a positive value even when costRatio is negative. This leads to users potentially receiving funds from the contract when they should actually be paying into the market.

proceedsBeforeFees = _calcCost(market, isPositive, false, votesToSell);

The incorrect calculation results in marketFunds[profileId] being decreased by the (incorrectly positive) proceedsBeforeFees, which can cause the contract's total balance to fall below the correct amount, violating financial invariants and potentially leading to loss of funds.

Internal Pre-conditions

No response

External Pre-conditions

No response

Attack Path

An attacker can exploit this flaw by selling votes in a scenario where the cost difference (costDiff) is negative. Due to the absolute value handling in _calcCost, the proceeds are incorrectly calculated as a positive amount, allowing the attacker to receive ETH from the contract when they actually should be paying into it.

Example Steps:

  1. Market Manipulation:

    • The attacker manipulates the market to create conditions where selling their votes results in a negative costDiff. This could involve purchasing a large number of opposing votes to shift the market state.

  2. Initiate Exploit:

    • The attacker calls sellVotes to sell their votes.

  3. Incorrect Proceeds Calculation:

    • The _calcCost function computes costRatio as negative due to the market state but then converts it to a positive value using the absolute value, leading to an incorrect positive proceedsBeforeFees.

  4. Withdrawal of Excess Funds:

    • The attacker receives ETH from the contract, withdrawing more funds than appropriate and potentially draining the contract's balance.

Impact

The attacker receives ETH from the contract when they should have been required to pay, improperly withdrawing funds.

PoC

Initial Market State

Assume the following initial state of the market:

  • Market Parameters:

    uint256 yesVotes = 2000;
uint256 noVotes = 1000;
uint256 liquidityParameter = 1000;
uint256 basePrice = 1 ether;

  • Attacker's Holding:

    uint256 attackerYesVotes = 1000; // Attacker owns 1000 "yes" votes

Market Manipulation

The attacker manipulates the market to create a condition where selling their "yes" votes results in a negative cost difference.

  • Market Shift:
    • The attacker (or colluding parties) buys additional "no" votes to significantly change the market dynamics.
    • New Market State: 
      yesVotes = 2000;
noVotes = 5000; // Increased from 1000 to 5000
// The attacker's holdings remain the same

Exploit Execution

The attacker proceeds to sell their "yes" votes by calling the sellVotes function.

// Attacker calls sellVotes to sell 1000 "yes" votes
reputationMarket.sellVotes(
    profileId,      // The profile ID of the market
    true,           // isPositive = true (selling "yes" votes)
    1000,           // votesToSell
    0               // minimumVotePrice (set to 0 for simplicity)
);

Incorrect Proceeds Calculation

Within the sellVotes function, the proceeds are calculated using the _calcCost function, which improperly handles negative cost differences.

Step-by-Step Calculation:

  1. Calculate Proceeds Before Fees:

    proceedsBeforeFees = _calcCost(market, isPositive, false, votesToSell);

  2. Inside _calcCost:

    • Determine Vote Delta for Selling Votes:

      // Since isPositive is true and isBuy is false, we are selling "yes" votes
voteDelta[0] = market.votes[TRUST] - amount;    // Updated "yes" votes
voteDelta[1] = market.votes[DISTRUST];          // "No" votes remain the same

      After selling:

      voteDelta[0] = 2000 - 1000 = 1000;
voteDelta[1] = 5000;

    • Calculate Cost Difference Using LMSR:

      int256 costRatio = LMSR.getCost(
    market.votes[TRUST],           // Current "yes" votes: 2000
    market.votes[DISTRUST],        // Current "no" votes: 5000
    voteDelta[0],                  // Updated "yes" votes: 1000
    voteDelta[1],                  // Updated "no" votes: 5000
    market.liquidityParameter      // Liquidity parameter: 1000
);

    • In LMSR.getCost Function: The cost difference (costDiff) is calculated as:

      costDiff = newCost - oldCost;

      Because the market has shifted significantly, newCost is less than oldCost, resulting in a negative costDiff.

    • Incorrect Absolute Value Handling:

      uint256 positiveCostRatio = costRatio > 0 ? uint256(costRatio) : uint256(-costRatio);

      Here, even though costRatio is negative, taking the absolute value makes positiveCostRatio positive.

    • Calculate Cost:

      cost = positiveCostRatio.mulDiv(
    market.basePrice,  // basePrice = 1 ether
    1e18,
    isPositive ? Math.Rounding.Floor : Math.Rounding.Ceil
);

      This results in a positive cost, which is incorrect because the cost difference should be negative.

  3. Back to sellVotes:

    • Proceeding with Incorrect Proceeds:

      proceedsBeforeFees = /* incorrect positive value */;

    • Calculate Fees: Fees are calculated based on the incorrectly positive proceedsBeforeFees.

    • Update Market Funds and User's Vote Holdings:

      markets[profileId].votes[TRUST] -= votesToSell;     // Deduct sold votes
votesOwned[msg.sender][profileId].votes[TRUST] -= votesToSell;

marketFunds[profileId] -= proceedsBeforeFees;       // Decrease market funds incorrectly

    • Transfer Funds to Attacker:

      _sendEth(proceedsAfterFees);  // Attacker receives ETH

Outcome

As a result of the incorrect cost calculation:

  • The attacker receives ETH from the contract when, in reality, they should have paid ETH into the market due to the negative cost difference.
  • The contract's marketFunds decreases improperly, potentially leading to a deficit.
  • This allows the attacker to withdraw excess funds from the contract.

Mitigation

Modify the _calcCost function to return an int256 representing the actual cost difference, preserving its sign, and adjust the calculation accordingly.

function _calcCost(
    Market memory market,
    bool isPositive,
    bool isBuy,
    uint256 amount
) private pure returns (int256 cost) {
    // Calculate the vote delta
    uint256[] memory voteDelta = new uint256[](2);
    if (isBuy) {
        // ... existing logic ...
    } else {
        // ... existing logic ...
    }

    // Get the cost difference from the LMSR
    int256 costRatio = LMSR.getCost(
        market.votes[TRUST],
        market.votes[DISTRUST],
        voteDelta[0],
        voteDelta[1],
        market.liquidityParameter
    );

    // Multiply cost ratio by base price to get cost
    cost = int256(market.basePrice).mulDiv(
        costRatio,
        1e18
    );
}

  • Adjust Calling Functions to Handle Negative Proceeds:

    In the sellVotes function, check if the proceedsBeforeFees is negative. If it is, handle the case appropriately, such as requiring the seller to pay into the market or preventing the sale.

    int256 proceedsBeforeFees = _calcCost(market, isPositive, false, votesToSell);
if (proceedsBeforeFees <= 0) {
    // The seller should not receive funds; optionally, they may need to pay in
    revert InvalidSellOperation("Proceeds cannot be negative");
}

uint256 proceeds = uint256(proceedsBeforeFees);

// Proceed with fee deductions and ETH transfer