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

missing check for equal length in initialize function can lead to unexpected issues #128

Open
hats-bug-reporter bot opened this issue Sep 29, 2024 · 3 comments
Labels
bug Something isn't working invalid This doesn't seem right

Comments

@hats-bug-reporter
Copy link

Github username: --
Twitter username: --
Submission hash (on-chain): 0x7a7d773b03b9201dd5513e49aa5ccd9cf6e74db85bb4f5938b73eeaf92a574cf
Severity: low

Description:
Description
the Market.sol is designed to manage prediction markets and here the initialize function is used to initialize the market with provided parameters but here the function does not check length of arrays i.e wrapped1155[] and data[].the issue arises here is if the length of both the arrays are not equal then as the function cannot be called again to change the array.owner cannot change the arrays too.

 function initialize(
        string memory _marketName,
        string[] memory _outcomes,
        uint256 _lowerBound,
        uint256 _upperBound,
        ConditionalTokensParams memory _conditionalTokensParams,
        RealityParams memory _realityParams,
        RealityProxy _realityProxy
    ) external {//@check-does not check the length is same
        require(!initialized, "Already initialized.");

        marketName = _marketName;
        outcomes = _outcomes;
        lowerBound = _lowerBound;
        upperBound = _upperBound;
        conditionalTokensParams = _conditionalTokensParams;//@audit-does not check for same length
        realityParams = _realityParams;
        realityProxy = _realityProxy;

        initialized = true;
    }

this issues directly affects other function like

function wrappedOutcome(uint256 index) external view returns (IERC20 wrapped1155, bytes memory data) {
        return (conditionalTokensParams.wrapped1155[index], conditionalTokensParams.data[index]);
    }

Attack Scenario
Describe how the vulnerability can be exploited.

Attachments

  1. Proof of Concept (PoC) File
  1. Revised Code File (Optional)

it is recommended to check the initialization parameters perfectly as the function can only be called once

@hats-bug-reporter hats-bug-reporter bot added the bug Something isn't working label Sep 29, 2024
@Tobi0x18
Copy link

For all markets created by MarketFactory, the conditionalTokensParams.wrapped1155 and conditionalTokensParams.data variables has the same length from L420-L421.

File: contracts\src\MarketFactory.sol
301:         (IERC20[] memory wrapped1155, bytes[] memory data) =
302:             deployERC20Positions(parentCollectionId, conditionId, config.outcomeSlotCount, params.tokenNames);


412:     function deployERC20Positions( 
413:         bytes32 parentCollectionId,
414:         bytes32 conditionId,
415:         uint256 outcomeSlotCount,
416:         string[] memory tokenNames
417:     ) internal returns (IERC20[] memory wrapped1155, bytes[] memory data) {
             [...] 
420:         wrapped1155 = new IERC20[](outcomeSlotCount);   //@info 
421:         data = new bytes[](outcomeSlotCount);           //@info
             [...]
440:     }

@greenlucid
Copy link

Security != Safety. Frontend should not allow creating markets with mismatched array lengths.

Out of Scope

Issues where the user is harming itself by interacting improperly with the contracts. It's possible for the users to call functions with improper parameters, incompatible tokens, etc but we assume that functions are called properly similarly to how they are called in the frontend. It's the job of smart contracts to protect against malicious behaviours, but it's the job of the frontend to protect against stupid ones!

@clesaege
Copy link

Those 2 arrays are set there and have the same length. Markets are to be deployed by the factory, not directly.

@clesaege clesaege added the invalid This doesn't seem right label Sep 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working invalid This doesn't seem right
Projects
None yet
Development

No branches or pull requests

3 participants