sherlock-audit / 2023-04-hubble-exchange-judging

7 stars 6 forks source link

0x3e84fa45 - Increasing minSizes will freeze positions #222

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

0x3e84fa45

medium

Increasing minSizes will freeze positions

Summary

Hubble governance can set a parameter called minSizes to limit the the minimum trading size accepted on the market. However, increasing or decreasing this parameter by the wrong fraction can result in permanent freezing of positions.

Vulnerability Detail

Hubble Protocol has two similar parameters:

The comments in the code suggest that minSizeRequirements should be equal to minSizes and is only stored in the OrderBook for cheaper assertions.

However, this assumption is incorrect, as increasing minSizes will freeze old positions.

  1. User creates a position with baseAssetQuantity = minSizes = minSizeRequirement set to 100.
  2. Government increases both minSizeRequirement and minSizes to 200. This means that future positions and trades must have a trade size of at least 200.
  3. The user cannot create an order to decrease their position because 100 is not a multiple of 200 (100 % 200 != 0). Similarly, they cannot be liquidated.

On the other hand, changing minSizeRequirement only affects future positions

Impact

The user's position cannot be liquidated, and they have no way of realizing any loss. Their position will remain indefinitely, and they can withdraw potential unrealized profit and funding payments from the vault.

Code Snippet

https://github.com/sherlock-audit/2023-04-hubble-exchange/blob/main/hubble-protocol/contracts/orderbooks/OrderBook.sol#L124 https://github.com/sherlock-audit/2023-04-hubble-exchange/blob/main/hubble-protocol/contracts/orderbooks/OrderBook.sol#L512-L514

Tool used

Manual Review

Recommendation

  1. Rename the variable minSizes in the OrderBook, as it does not have a one-to-one relation to the AMM variable minSizeRequirement
  2. Enforce that the new parameter is smaller than the previous one and is a multiple of it.
function updateMinSize(uint ammIndex, int minSize) external onlyGovernance {
+       isMultiple(minSizes[ammIndex], minSize);
    minSizes[ammIndex] = minSize;
}