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

Upbeat Coffee Badger - Using isPositive to determine rounding in _calcCost(…) instead of isBuy (or a similar variable) can lead to a 1 wei discrepancy during buy/sell operations. #145

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

Comments

@sherlock-admin3
Copy link
Contributor

Upbeat Coffee Badger

High

Using isPositive to determine rounding in _calcCost(…) instead of isBuy (or a similar variable) can lead to a 1 wei discrepancy during buy/sell operations.

Summary

https://github.com/sherlock-audit/2024-12-ethos-update/blob/c3a2b007d0ddfcb476f300f8b766808f0e3e2dfd/ethos/packages/contracts/contracts/ReputationMarket.sol#L1017
A subtle but important bug lurks in _calcCost(…) at the very end where it applies rounding based on isPositive instead of isBuy. In other words, it uses:

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

But whether we should round up or down actually depends on whether the user is buying or selling, not whether they’re buying “trust” (positive) or “distrust” (negative). By keying off of isPositive, the code can produce unintended outcomes.

Why it’s a bug
1. When buying (isBuy = true)
• Generally, the buyer wants an accurate (and slightly rounded up) figure to ensure they pay at least as much as the bonding curve dictates.
2. When selling (isBuy = false)
• The seller wants an accurate (and slightly rounded down) figure to ensure they don’t get overcharged on slippage.

Instead, the code is doing:

If you’re buying/selling trust (isPositive = true), do Math.Rounding.Floor;
If you’re buying/selling distrust (isPositive = false), do Math.Rounding.Ceil.

That logic was presumably intended to ensure the prices of “trust” plus “distrust” always total the basePrice. However, by mixing up which side of the trade you’re on (buy vs. sell) with whether the trade is on the trust or distrust side, the final cost or proceeds for each trade can be off by 1 wei in the wrong direction, causing price or revenue mismatches over many transactions.

Root Cause

No response

Internal Pre-conditions

No response

External Pre-conditions

No response

Attack Path

No response

Impact

No response

PoC

No response

Mitigation

Inside _calcCost(…), the last line should decide rounding based on isBuy rather than isPositive.

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