hats-finance / SeeR-PM-0x899bc13919880db76edf4ccd72bdfa5dfa666fb7

1 stars 0 forks source link

`MarketFacotry::createCategoricalMarket`, `MarketFacotry::createMultiCategoricalMarket`, `MarketFacotry::createScalarMarket`, & `MarketFacotry::createMultiScalarMarket` does not account for a market outcome of 2 different outcomes in the `outcomes` array #65

Open hats-bug-reporter[bot] opened 1 month ago

hats-bug-reporter[bot] commented 1 month ago

Github username: @a12jmx Twitter username: a12jmx Submission hash (on-chain): 0xdc7536ba44de0140420850294b1b484ca657cd3c7e1933822b75e195e67b4010 Severity: high

Description: Description\

Note: Highlighted words with a => in front of them links directly to the code if clicked.

The functions MarketFacotry::createCategoricalMarket, MarketFacotry::createMultiCategoricalMarket, MarketFacotry::createScalarMarket, & MarketFacotry::createMultiScalarMarket all use the same array which gets set in a struct => CreateMarketParams

        string[] outcomes;

This array adds up strings of relevant outcomes in a prediction.

Basic example, for instance:

Question: "Insert anything?"

Outcome 1: Yes

Outcome 2: No

or,

Question: "Insert anything possible?"

Outcome 1: Yes

Outcome 2: No

Outcome 3: Only if (insert certain params to meet)

Looking at the code, 3 functions intend to proceed in execution only if the outcomes array is filled with 2 or more outcomes:

=> MarketFacotry::createCategoricalMarket

        require(params.outcomes.length >= 2, "Outcomes count must be 2 or more");

=> MarketFacotry::createMultiCategoricalMarket

        require(params.outcomes.length >= 2, "Outcomes count must be 2 or more");

=> MarketFacotry::createMultiScalarMarket

        require(params.outcomes.length >= 2, "Outcomes count must be 2 or more");

One function only proceeds to execute if the outcomes array have only 2 outcomes:

=> MarketFacotry::createScalarMarket

        require(params.outcomes.length == 2, "Outcomes count must be 2");

The intention is to only follow through with these functions if the outcome count in the outcomes array have at least 2 outcomes filled in all scenarios. Technically, it only starts checking at the 3rd position of the outcomes array within these functions.

Array indexes start at 0, unlike normal counting where you typically begin at 1. So the 1st item is the 0th, the 2nd item is the 1st, the 3rd item is the 4th, etc… So an array with 4 items would only go to the 3rd index.

So, briefly:

Array position 0 will == real position 1

Array position 1 will == real position 2

Array position 2 will == real position 3

Array position 3 will == real position 4

This leads to, if there are 2 outcomes, i.e. Yes or No, which are extremely popular outcomes in predication markets, the outcomes will never occur on a smart contract level and essentially, whoever predicted will have their collateral stuck as the outcome never technically follows through.

Impact\

Classifying this as a High vulnerability for the following reasons:

  1. No external or internal malicious actions need to occur for this to take effect.

  2. This will happen every time there are a minimum of 2 outcomes.

  3. The protocol, at a fundamental level offers prediction markets, and 2 outcomes such as Yes or No as an example are incredibly prevelant in prediction markets.

  4. Seer has 4 different types of prediction markets in reference to => Key Components:Prediction market: in it's docs:

    Categorical Markets: Select one out of multiple outcomes.

    Multi-Categorical Markets: Similar to categorical market but allowing selection of multiple outcomes.

    Scalar Markets: Numeric outcome within a specified range.

    Multi-Scalar Markets: Multiple numeric outcomes.

These prediction types are the 4 functions outlayed above which gets effected where the 2 outcomes can be techincally anything in regards to it's specific prediction type with the array being a string, on a coding level, anything that has double qoutions arround it, i.e. "quote" or "1", is a string, but this will not change the lack of execution of the functions above mentioned.

  1. If the prediction market has only 2 outcomes, with any of the above mentioned functions, the collateral of users who participated with will be technically stuck, as the functions will never complete on a smart contract level.

Attachments

Revised Code File (Optional)

Given arrays start at the 0th position, the checks to see if the outcomes array has at least 2 values should be revised down from a check of 2 to a check of 1 as follows:

=> MarketFacotry::createCategoricalMarket

    function createCategoricalMarket(CreateMarketParams calldata params) external returns (address) {
--      require(params.outcomes.length >= 2, "Outcomes count must be 2 or more");
++      require(params.outcomes.length >= 1, "Outcomes count must be 2 or more");

        string[] memory encodedQuestions = new string[](1);

=> MarketFacotry::createMultiCategoricalMarket

    function createMultiCategoricalMarket(CreateMarketParams calldata params) external returns (address) {
--      require(params.outcomes.length >= 2, "Outcomes count must be 2 or more");
++      require(params.outcomes.length >= 1, "Outcomes count must be 2 or more");

        string[] memory encodedQuestions = new string[](1);

=> MarketFacotry::createScalarMarket

        require(params.upperBound < type(uint256).max - 2, "upperBound must be less than uint256.max - 2");
--      require(params.outcomes.length == 2, "Outcomes count must be 2");
++      require(params.outcomes.length == 2, "Outcomes count must be 2");

=> MarketFacotry::createMultiScalarMarket

 function createMultiScalarMarket(CreateMarketParams calldata params) external returns (address) {
--      require(params.outcomes.length >= 2, "Outcomes count must be 2 or more");
++      require(params.outcomes.length >= 1, "Outcomes count must be 2 or more");

        string[] memory encodedQuestions = new string[](params.outcomes.length);
greenlucid commented 1 month ago

Wtf You gotta look up the meaning of length bro

clesaege commented 1 month ago

I can't really understand this report, and as @greenlucid pointed out, the most plausive explanation for this report is that the hunter is confused to the way arrays work in solidity.