Mean Brick Mongoose
Medium
the removeMarketConfig function allows an admin to remove a market. If the market is not the last index, it is swapped with the last index market and then removed. The problem occurs when a malicious validator can backrun a user's tx in order for the user to create a market he did not intend.
In ReputationMarket.sol ln 388 https://github.com/sherlock-audit/2024-12-ethos-update/blob/c3a2b007d0ddfcb476f300f8b766808f0e3e2dfd/ethos/packages/contracts/contracts/ReputationMarket.sol#L388
function removeMarketConfig(uint256 configIndex) public onlyAdmin whenNotPaused {
// Cannot remove if only one config remains
if (marketConfigs.length <= 1) revert InvalidMarketConfigOption("Must keep one config");
// Check if the index is valid
if (configIndex >= marketConfigs.length) revert InvalidMarketConfigOption("index not found");
emit MarketConfigRemoved(configIndex, marketConfigs[configIndex]);
// If this is not the last element, swap with the last element
uint256 lastIndex = marketConfigs.length - 1;
if (configIndex != lastIndex) {
marketConfigs[configIndex] = marketConfigs[lastIndex];
}
// Remove the last element
marketConfigs.pop();
}
The admin can remove a market, this is done by swapping the index with the last index and popping the last index. The problem occurs when a malicious validator sees a user who is attempting to create a market which is also going to be removed in the same block by the admin. In normal execution the user would have created the market using the config, then the admin tx would be executed and the market config index would be deleted from the list. However a malicious validator can reorganize the block and back run the user's transaction to occur right after the admins removal transaction. In this scenario 2 thing could happen. The market created has a lower creation cost and the user is refunded the excess eth. Or the user sent extra eth to be safe and the eth is consumed by a larger creation cost market.
In each scenario the user is forced to take an action unwanted by him. Because the function removeMarketConfig has the whenNotPaused modifier, it must be called during the unpaused state which causes this issue.
- normal user must attempt to create a market which has a market config index that is not the last one in the list
- admin must have also submit a transaction in the same block to remove said index.
No response
- malicious validator sees in the mempool a user attempting to create a market with market config index that is not the last market config
- Malicious validator also sees on the mempool a transaction to remove the same market config.
- malicious user backruns the users tx to come after the admin tx
- User is forced to pay and create a market he did not desire.
User is forced to create a market he did not desire. There is 2 outcomes, 1 the different market config features a lower creation cost and he is refunded the excess eth. Or the market config features a more expensive creation cost and the user is charged a higher amount which would be taken from him in the chance he sent excess eth. The second scenario could be considered user error but the first scenario is not.
No response
Only allow market configs to be deleted during the paused state.