-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
- Loading branch information
1 parent
9c27e73
commit 5bb1ee9
Showing
156 changed files
with
14,935 additions
and
10 deletions.
There are no files selected for viewing
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,96 @@ | ||
Fluffy Charcoal Toad | ||
|
||
High | ||
|
||
# LMSR Price Manipulation via Inconsistent Rounding | ||
|
||
### Summary | ||
|
||
Inconsistent rounding modes in price calculations will cause a financial loss for the protocol and market participants as an attacker can exploit price differences through sequential buy/sell operations. | ||
|
||
### Root Cause | ||
|
||
In `ReputationMarket.sol:_calcVotePrice` the rounding modes are inconsistent between trust (Floor) and distrust (Ceil) votes: | ||
|
||
https://github.com/sherlock-audit/2024-12-ethos-update/blob/main/ethos/packages/contracts/contracts/ReputationMarket.sol#L992 | ||
|
||
```javascript | ||
return odds.mulDiv( | ||
market.basePrice, | ||
1e18, | ||
isPositive ? Math.Rounding.Floor : Math.Rounding.Ceil | ||
); | ||
``` | ||
|
||
### Internal Pre-conditions | ||
|
||
1. Market needs to have active trading (not graduated) | ||
2. Market needs to have sufficient liquidity for trades | ||
3. Market votes need to be at relatively low numbers where rounding impact is significant | ||
|
||
### External Pre-conditions | ||
|
||
_No response_ | ||
|
||
### Attack Path | ||
|
||
1. Attacker calls `buyVotes(profileId, true, largeAmount) `to buy trust votes at floor-rounded (lower) price | ||
2. Attacker immediately calls `sellVotes(profileId, true, largeAmount)` to sell at ceiling-rounded (higher) price | ||
3. Attacker repeats steps 1-2 to maximize profit | ||
4. Each cycle generates profit from the rounding difference | ||
|
||
### Impact | ||
|
||
The protocol suffers continuous losses proportional to the volume of trades. Attacker gains the difference between ceiling and floor rounded prices on each cycle. At low vote counts, this can represent a significant percentage of trade value. | ||
|
||
### PoC | ||
|
||
```javascript | ||
function testLMSRManipulation() public { | ||
// Setup market with low votes | ||
uint256 profileId = 1; | ||
uint256 initialVotes = 100; | ||
market.createMarket{value: 1 ether}(); | ||
|
||
// Initial state | ||
uint256 attackerBalance = address(this).balance; | ||
|
||
// Execute attack cycle | ||
for(uint i = 0; i < 5; i++) { | ||
uint256 buyPrice = market.getVotePrice(profileId, true); | ||
market.buyVotes{value: buyPrice * 10}(profileId, true, 10); | ||
|
||
uint256 sellPrice = market.getVotePrice(profileId, true); | ||
market.sellVotes(profileId, true, 10); | ||
|
||
// Profit from each cycle | ||
uint256 profit = sellPrice - buyPrice; | ||
console.log("Profit from cycle:", profit); | ||
} | ||
|
||
assertGt(address(this).balance, attackerBalance); | ||
} | ||
``` | ||
|
||
### Mitigation | ||
|
||
1. Use consistent rounding for both trust and distrust | ||
```javascript | ||
function _calcVotePrice(Market memory market, bool isPositive) private pure returns (uint256) { | ||
require(market.votes[TRUST] >= MIN_VOTES && market.votes[DISTRUST] >= MIN_VOTES, | ||
"Below minimum votes"); | ||
|
||
uint256 odds = LMSR.getOdds( | ||
market.votes[TRUST], | ||
market.votes[DISTRUST], | ||
market.liquidityParameter, | ||
isPositive | ||
); | ||
|
||
return odds.mulDiv(market.basePrice, 1e18, Math.Rounding.Floor); | ||
} | ||
``` | ||
|
||
2. Add minimum vote thresholds to reduce impact of rounding | ||
3. Implement trade size limits | ||
4. Add cooldown period between buy and sell operations |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,82 @@ | ||
Fluffy Charcoal Toad | ||
|
||
High | ||
|
||
# Insufficient Slippage Protection in Vote Purchases | ||
|
||
### Summary | ||
|
||
Missing slippage validation on final vote count will cause financial losses for users as attackers can front-run large trades to force unfavorable execution prices. `buyVotes` decreases votes until they fit within msg.value without checking minVotesToBuy. | ||
|
||
### Root Cause | ||
|
||
In `ReputationMarket.sol:buyVotes` there is a missing validation check between the final number of votes purchased and the user's specified minimum: | ||
|
||
https://github.com/sherlock-audit/2024-12-ethos-update/blob/main/ethos/packages/contracts/contracts/ReputationMarket.sol#L440 | ||
|
||
```javascript | ||
// if the cost is greater than the maximum votes to buy, | ||
// decrement vote count and recalculate until we identify the max number of votes they can afford | ||
while (totalCostIncludingFees > msg.value) { | ||
currentVotesToBuy--; | ||
(purchaseCostBeforeFees, protocolFee, donation, totalCostIncludingFees) = _calculateBuy( | ||
markets[profileId], | ||
isPositive, | ||
currentVotesToBuy | ||
); | ||
``` | ||
### Internal Pre-conditions | ||
1. User needs to submit a buyVotes transaction with maxVotesToBuy significantly higher than minVotesToBuy | ||
2. Transaction needs to include sufficient ETH for minVotesToBuy at current prices | ||
3. Market needs to have enough liquidity to execute large trades | ||
### External Pre-conditions | ||
1. Network needs to have sufficient congestion to allow front-running | ||
### Attack Path | ||
1. Attacker monitors mempool for large buyVotes transactions | ||
2. When victim transaction detected with large maxVotesToBuy, attacker submits front-running trade with higher gas price | ||
3. Attacker's trade executes first, driving up market price | ||
4. Victim's transaction executes but receives fewer votes than minVotesToBuy due to price impact | ||
5. Attacker can then sell votes at higher price | ||
### Impact | ||
Users suffer losses due to unfavorable trade execution, potentially receiving significantly fewer votes than their specified minimum. | ||
### PoC | ||
```javascript | ||
function testSlippageExploit() public { | ||
// Setup market | ||
uint256 profileId = 1; | ||
market.createMarket{value: 1 ether}(); | ||
|
||
// Victim transaction parameters | ||
uint256 maxVotes = 1000; | ||
uint256 minVotes = 800; | ||
uint256 victimETH = 10 ether; | ||
|
||
// Attacker front-runs | ||
vm.prank(attacker); | ||
market.buyVotes{value: 20 ether}(profileId, true, 2000); | ||
|
||
// Victim transaction executes | ||
vm.prank(victim); | ||
market.buyVotes{value: victimETH}(profileId, true, maxVotes, minVotes); | ||
|
||
// Check victim's received votes | ||
uint256 actualVotes = market.getUserVotes(victim, profileId).trustVotes; | ||
assertLt(actualVotes, minVotes); // Victim receives fewer votes than minimum | ||
} | ||
``` | ||
### Mitigation | ||
1. Add slippage check before executing trade | ||
2. Add deadline parameter to prevent stale trades | ||
3. Implement price oracle with TWAP to reduce manipulation |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,53 @@ | ||
Wide Bone Shetland | ||
|
||
Medium | ||
|
||
# withdrawGraduatedMarketFunds and withdrawDonations functions should avoid using the whenNotPaused modifier | ||
|
||
### Summary | ||
|
||
Using the whenNotPaused modifier will cause users to lose access to their funds when the contract is paused, as it prevents the withdrawGraduatedMarketFunds and withdrawDonations functions from being executed. This could leave users unable to withdraw their funds/donations during emergencies or unexpected pauses, | ||
|
||
### Root Cause | ||
|
||
The root cause lies in the use of the whenNotPaused modifier in the withdrawDonations and withdrawGraduatedMarketFunds functions. | ||
|
||
"withdrawDonations" | ||
|
||
https://github.com/sherlock-audit/2024-12-ethos-update/blob/c3a2b007d0ddfcb476f300f8b766808f0e3e2dfd/ethos/packages/contracts/contracts/ReputationMarket.sol#L651 | ||
|
||
|
||
"withdrawGraduatedMarketFunds" | ||
|
||
https://github.com/sherlock-audit/2024-12-ethos-update/blob/c3a2b007d0ddfcb476f300f8b766808f0e3e2dfd/ethos/packages/contracts/contracts/ReputationMarket.sol#L740 | ||
|
||
|
||
### Internal Pre-conditions | ||
|
||
1. The contract must be paused, either by a malicious or a compromised owner, for the vulnerability path to occur. | ||
|
||
2. The withdrawDonations and withdrawGraduatedMarketFunds functions includes the whenNotPaused modifier, preventing withdrawals while the contract is paused. | ||
|
||
3. Users need to have funds available in the contract to attempt withdrawal. | ||
|
||
4. The paused state must last long enough for users to try to withdraw funds, thereby triggering the blockage (whenNotPaused). | ||
|
||
### External Pre-conditions | ||
|
||
_No response_ | ||
|
||
### Attack Path | ||
|
||
_No response_ | ||
|
||
### Impact | ||
|
||
- The User Loses Access to Funds: | ||
|
||
### PoC | ||
|
||
_No response_ | ||
|
||
### Mitigation | ||
|
||
Provide an `emergencyWithdrawFunds and emergencyWithdrawDonations` method allowing users to withdraw their funds and donations when the protocol is paused or remove the whenNotPaused modifier from the affected functions. This change should be carefully reviewed and tested to ensure it does not introduce other security risks. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,111 @@ | ||
Loud Onyx Perch | ||
|
||
Medium | ||
|
||
# Wrong vote price/cost will be returned due to inversed rounding condition | ||
|
||
### Summary | ||
|
||
When the protocol calculates a vote price, it computes the `costRatio` then which is divided by the price's base price, so some rounding should be made here, currently, the protocol rounds the answer down for `TRUST` votes: | ||
```typescript | ||
cost = positiveCostRatio.mulDiv( | ||
market.basePrice, | ||
1e18, | ||
isPositive ? Math.Rounding.Floor : Math.Rounding.Ceil | ||
); | ||
``` | ||
|
||
On the other hand, in `_calcVotePrice`, which is used for the same goal but represents a midpoint between the next marginal transaction, the comment clearly states that the answer should be rounded up for `TRUST` votes: | ||
```typescript | ||
function _calcVotePrice(Market memory market, bool isPositive) private pure returns (uint256) { | ||
// odds are in a ratio of N / 1e18 | ||
uint256 odds = LMSR.getOdds( | ||
market.votes[TRUST], | ||
market.votes[DISTRUST], | ||
market.liquidityParameter, | ||
isPositive | ||
); | ||
// multiply odds by base price to get price; divide by 1e18 to get price in wei | ||
@> // round up for trust, down for distrust so that prices always equal basePrice | ||
return | ||
odds.mulDiv(market.basePrice, 1e18, isPositive ? Math.Rounding.Floor : Math.Rounding.Ceil); | ||
} | ||
``` | ||
|
||
This results in misleading vote costs being returned to the users. | ||
|
||
### Root Cause | ||
|
||
Inversed rounding conditions in both `_calcVotePrice` and `_calcCost`: | ||
```typescript | ||
odds.mulDiv(market.basePrice, 1e18, isPositive ? Math.Rounding.Floor : Math.Rounding.Ceil); | ||
``` | ||
|
||
https://github.com/sherlock-audit/2024-12-ethos-update/blob/main/ethos/packages/contracts/contracts/ReputationMarket.sol#L1001-L1003 | ||
https://github.com/sherlock-audit/2024-12-ethos-update/blob/main/ethos/packages/contracts/contracts/ReputationMarket.sol#L1054-L1058 | ||
|
||
### Internal Pre-conditions | ||
|
||
_No response_ | ||
|
||
### External Pre-conditions | ||
|
||
_No response_ | ||
|
||
### Attack Path | ||
|
||
_No response_ | ||
|
||
### Impact | ||
|
||
The wrong vote price/cost will be returned. | ||
|
||
### PoC | ||
|
||
Add the following test in `ethos/packages/contracts/test/reputationMarket/rep.fees.test.ts`: | ||
```typescript | ||
describe('PoC', () => { | ||
it('vote price rounding inveresed', async () => { | ||
const user1 = ethosUserA.signer; | ||
|
||
await reputationMarket | ||
.connect(user1) | ||
.buyVotes(DEFAULT.profileId, true, 1, 1, { value: DEFAULT.creationCost }); | ||
|
||
expect(await reputationMarket.getVotePrice(DEFAULT.profileId, true)).to.be.equal( | ||
5002499999791666n, // Rounded down | ||
); | ||
}); | ||
}); | ||
``` | ||
|
||
### Mitigation | ||
|
||
Inverse the rounding in both `_calcVotePrice` and `_calcCost`. | ||
|
||
```diff | ||
function _calcVotePrice(Market memory market, bool isPositive) private pure returns (uint256) { | ||
// ... | ||
|
||
// multiply odds by base price to get price; divide by 1e18 to get price in wei | ||
// round up for trust, down for distrust so that prices always equal basePrice | ||
- return odds.mulDiv(market.basePrice, 1e18, isPositive ? Math.Rounding.Floor : Math.Rounding.Ceil); | ||
+ return odds.mulDiv(market.basePrice, 1e18, isPositive ? Math.Rounding.Ceil : Math.Rounding.Floor); | ||
} | ||
|
||
function _calcCost( | ||
Market memory market, | ||
bool isPositive, | ||
bool isBuy, | ||
uint256 amount | ||
) private pure returns (uint256 cost) { | ||
// ... | ||
|
||
cost = positiveCostRatio.mulDiv( | ||
market.basePrice, | ||
1e18, | ||
- isPositive ? Math.Rounding.Floor : Math.Rounding.Ceil | ||
+ isPositive ? Math.Rounding.Ceil : Math.Rounding.Floor | ||
); | ||
} | ||
``` |
Oops, something went wrong.