hats-finance / SeeR-PM-0x899bc13919880db76edf4ccd72bdfa5dfa666fb7

1 stars 0 forks source link

Missing Validation for `lowerBound` and `upperBound` in the `Market` Contract leading to DOS #63

Open hats-bug-reporter[bot] opened 2 months ago

hats-bug-reporter[bot] commented 2 months ago

Github username: -- Twitter username: -- Submission hash (on-chain): 0xa2be774f8cb5c86358593e2338ab864e60d4ae0ee24be7b13c6a5f0a5aa8fbc3 Severity: medium

Description: Description\ In the current implementation of the Market contract, the initialize function allows users to set scalar market boundaries via the _lowerBound and _upperBound parameters. However, the contract does not validate that the _lowerBound value is less than or equal to the _upperBound value. This omission creates the potential for logical errors and underflows during the market resolution process.

Scalar markets are markets where the outcome is a numerical value within a predefined range, defined by lowerBound and upperBound. These bounds are crucial to the correct functioning of the market since they establish the valid range within which the actual result can fall.

The lack of validation in the initialize function can lead to scenarios where:

The problematic area is in the initialize function:

function initialize(
    string memory _marketName,
    string[] memory _outcomes,
    uint256 _lowerBound,
    uint256 _upperBound,
    ...
) external {
    require(!initialized, "Already initialized.");
    marketName = _marketName;
    outcomes = _outcomes;
    lowerBound = _lowerBound;
    upperBound = _upperBound;
    ...
    initialized = true;
}

The contract allows any values for lowerBound and upperBound, leading to a scenario where:

else {
    payouts[0] = high - answer;
    payouts[1] = answer - low;
}

Here, high - answer and answer - low could result in an underflow if lowerBound > upperBound due to incorrect assumptions about the relative values of answer, low, and high.

Impact Area:

This lack of validation directly affects the behavior of the scalar markets when the market is resolved, particularly in the resolveScalarMarket function of the RealityProxy contract. The payouts array might result in underflows or incorrect payouts depending on how the results compare to the bounds.

Impact\ If lowerBound > upperBound, the payout calculations in the resolveScalarMarket function will cause arithmetic underflows which will cause regular reverts leading to potential DOS.

payouts[0] = high - answer;
payouts[1] = answer - low;

If low is greater than high, these operations will fail, and the contract will throw an exception, reverting the entire transaction and leaving the market unresolved

Recommendation\ To mitigate this issue, a simple validation check should be added in the initialize function to ensure that _lowerBound is always less than _upperBound. This will prevent the creation of invalid scalar markets and avoid the underflows and logical inconsistencies that result from inverted bounds.

Suggested fix:

require(_lowerBound < _upperBound, "Lower bound must be less upper bound.");

This check will ensure that scalar markets always have a valid range, preventing runtime errors and maintaining the integrity of the market.

xyzseer commented 2 months ago

All Markets are created by the MarketFactory, and this is validated on createScalarMarket https://github.com/seer-pm/demo/blob/4e56254cbd071b6f678a108ccdb8660951636d27/contracts/src/MarketFactory.sol#L185

the Market itself is just a value object, and creating an invalid Market outside of the MarketFactory doesn't represent any risk.

clesaege commented 2 months ago

Yeah, this is checked at market creation.