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

Best Carbon Eagle - _createMarket could coincide with removeMarketConfig causing user to create market with a different config type #713

Open
sherlock-admin2 opened this issue Dec 5, 2024 · 0 comments

Comments

@sherlock-admin2
Copy link
Contributor

Best Carbon Eagle

High

_createMarket could coincide with removeMarketConfig causing user to create market with a different config type

Vulnerability Details

In ReputationMarket.sol,

removeMarketConfig removes a market config type template that the user can create. If the type it is trying to remove isnt the last element, it will shift the last element to the index it wants to remove, then pop the last element.

Importantly, it causes the indexes of config types to change.

Creating a market with an allowed user through createMarketWithConfig requires the whitelisted user to pass in just the index of the market config that needs to be created.

Hence, when these 2 functions are called around the same time, a user will end up creating a market config type that is different than intended and it will be permanent.

Proof Of Concept

Suppose current market types are:

  1. A (initial liquidity = 3 ether)
  2. B (initial liquidity = 3 ether)
  3. C (initial liquidity = 2 ether)

If removeMarketConfig(0) and createMarketWithConfig(0) are called around the same time:

removeMarketConfig(0) will put C at index 0.

Then createMarketWithConfig(0) will end up creating a market with C instead of reverting. (The extra 1 ether of msg.value provided (since 3-2=1) will just be refunded to the user via _sendEth)

Instead, the function should have reverted as if A was not available, the user of the profile may want to choose config type B instead.

Impact

Since votes[TRUST] and votes[DISTRUST] will be set to the initial votes that is >= 1. And it is impossible to sell to < 1.

votes will then forever be >= 1 for that profile and it becomes impossible to call create market with a different market config type for that profile ever again, (since it will js revert with market exists) hence this is permanent.

It will eventually cause fund loss for the user as market config types are meant to be chosen by the user to match the volatility that the user predicts their own market will be.

If this bug ends up force creating a different market config for the user that is less suitable for their expected volatility, then eventually voters who participate will have less than ideal fund profits/loss and the owner of the profile has to invest more money to stabilise and improve reputation.

Code snippet

https://github.com/sherlock-audit/2024-11-ethos-network-ii/blob/57c02df7c56f0b18c681a89ebccc28c86c72d8d8/ethos/packages/contracts/contracts/ReputationMarket.sol#L281-L293

Recommendation

When creating market, other than just passing in the index of the market config, add the actual market config as the parameter itself so that it will revert in this case.

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