Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Abundant Orchid Copperhead - Wrong rounding direction will lead to user can't sell the last votes in the market due to underflow #144

Open
sherlock-admin4 opened this issue Dec 30, 2024 · 0 comments

Comments

@sherlock-admin4
Copy link
Contributor

Abundant Orchid Copperhead

Medium

Wrong rounding direction will lead to user can't sell the last votes in the market due to underflow

Summary

Wrong rounding direction in _calcCost() will lead to the user not being able to sell the last votes in the market due to underflow

Root Cause

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

This calculation has wrong rounding:

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

Instead of rounding in favor of the protocol (aka rounding up when user buying, rounding down when user selling), the calculation will round down when buying/selling TRUST vote, and round up when buying/selling DISTRUST vote

Internal Pre-conditions

  1. Admin creates a market with no/very small amount of creation cost
  2. The base price of that market is mildly odd, which makes room for rounding

External Pre-conditions

None

Attack Path

  1. The initial market will be: vote of TRUST=1, vote of DISTRUST=1, basePrice = 1.111e18, liquidation parameter = 100, marketFunds = 0 because the admin can create markets with 0 amount of ether cost
  2. User A buy 100 DISTRUST -> _calcCost() = 68083609502231759384 ~ 68.08e18 -> marketFund = 68083609502231759384
  3. User A sell 9 DISTRUST -> _calCost() = 7200065779706726078 ~ 7.2e18 -> marketFund = 68083609502231759384 - 7200065779706726078 = 60883543722525033306
  4. User A sell 11 DISTRUST -> _calCost() = 7200065779706726078 ~ 7.2e18 ->marketFund = 60883543722525033306 - 8548283373986344155= 52335260348538689151
  5. User A sell 80 DISTRUST, drain the whole market -> _calCost() = 52335260348538689152 ~ 52.33e18 -> marketFund = 52335260348538689151 - 52335260348538689152 = -1. Because marketFund is uint256 type, the transaction revert
  6. Instead, user A sell 79 DISTRUST -> _calCost() = 51778371604325108785 ~ 51.78e18 -> marketFund = 52335260348538689151 - 51778371604325108785 = 556888744213580366 -> not reverted

Impact

As a result, user A can't sell the last DISTRUST vote, leading to losing ~0.55e18 ETH

PoC

No response

Mitigation

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

    cost = positiveCostRatio.mulDiv(
      market.basePrice,
      1e18,
-     isPositive ? Math.Rounding.Floor : Math.Rounding.Ceil
+     !isBuy ? Math.Rounding.Floor : Math.Rounding.Ceil
    );
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant