Market parameter is not validated in router when merge position

Github username: -- Twitter username: -- Submission hash (on-chain): 0x3404c9b7db7bcad46bc5b8af3a8a852bb2c99354f54cde83290b9ee3cdc43b90 Severity: high

Description: Description\ Describe the context and the effect of the vulnerability.

Attack Scenario\

https://github.com/hats-finance/SeeR-PM-0x899bc13919880db76edf4ccd72bdfa5dfa666fb7/blob/4e56254cbd071b6f678a108ccdb8660951636d27/contracts/src/Router.sol#L143

   function redeemPositions(IERC20 collateralToken, Market market, uint256[] calldata outcomeIndexes) public {
@        bytes32 parentCollectionId = market.parentCollectionId();
        uint256 initialBalance;

        if (parentCollectionId == bytes32(0)) {
            initialBalance = collateralToken.balanceOf(address(this));
        }

        _redeemPositions(collateralToken, market, outcomeIndexes);

        if (parentCollectionId == bytes32(0)) {
            uint256 finalBalance = collateralToken.balanceOf(address(this));

            if (finalBalance > initialBalance) {
                collateralToken.transfer(msg.sender, finalBalance - initialBalance);
            }
        }
    }

the input market is not validated.

the user can pass in a malicious market address to merge position and cause loss of fund.

https://github.com/hats-finance/SeeR-PM-0x899bc13919880db76edf4ccd72bdfa5dfa666fb7/blob/4e56254cbd071b6f678a108ccdb8660951636d27/contracts/src/Router.sol#L90

    function mergePositions(IERC20 collateralToken, Market market, uint256 amount) public {
        _mergePositions(collateralToken, market, amount);

 @       if (market.parentCollectionId() == bytes32(0)) {
            // send collateral tokens back to the user.
            collateralToken.transfer(msg.sender, amount);
        }
    }

    /// @notice Merges positions and receives the collateral tokens.
    /// @dev Callers to this function must send the collateral to the user.
    /// @param collateralToken The address of the ERC20 used as collateral.
    /// @param market The Market to merge.
    /// @param amount The amount of outcome tokens to merge.
    function _mergePositions(IERC20 collateralToken, Market market, uint256 amount) internal {
  @      bytes32 parentCollectionId = market.parentCollectionId();
  @      bytes32 conditionId = market.conditionId();

        uint256[] memory partition = getPartition(conditionalTokens.getOutcomeSlotCount(conditionId));

        // we need to unwrap the outcome tokens because they will be burnt during the merge.

        for (uint256 j = 0; j < partition.length; j++) {
            uint256 tokenId = getTokenId(collateralToken, parentCollectionId, conditionId, partition[j]);

  @          (IERC20 wrapped1155, bytes memory data) = market.wrappedOutcome(j);

            wrapped1155.transferFrom(msg.sender, address(this), amount);
            wrapped1155Factory.unwrap(address(conditionalTokens), tokenId, amount, address(this), data);
        }

        conditionalTokens.mergePositions(address(collateralToken), parentCollectionId, conditionId, partition, amount);

        if (parentCollectionId != bytes32(0)) {
            // it's merging from a parent position, so we need to wrap these tokens and send them back to the user.
            uint256 tokenId = conditionalTokens.getPositionId(address(collateralToken), parentCollectionId);

            (IERC20 wrapped1155, bytes memory data) = market.parentWrappedOutcome();

            // wrap to erc20.
            conditionalTokens.safeTransferFrom(address(this), address(wrapped1155Factory), tokenId, amount, data);

            // transfer the ERC20 back to the user.
            wrapped1155.transfer(msg.sender, amount);
        }
    }

all external query to market contract is labeled with @

a malicious market contract can be

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;

contract MaliciousMarket {

    // Unique identifier for the parent collection
    function parentCollectionId() public view returns (bytes32) {
        // Implementation needed
        return bytes32(0); // Placeholder
    }

    // Unique identifier for the condition
    function conditionId() public view returns (bytes32) {
        // Implementation needed
        return bytes32(0); // Placeholder
    }

    // Returns the wrapped outcome
    function wrappedOutcome() public view returns (address token, bytes memory data) {
        // Implementation needed
        return (address(0), ""); // Placeholder
    }

    // Returns the parent wrapped outcome
    function parentWrappedOutcome() public view returns (address token, bytes memory data) {
        // Implementation needed
        return (address(0), ""); // Placeholder
    }

    // Additional logic can be added here
}

for example, the user can make a market with no parentCollectionId return a parentCollectionId that comes from another market, etc..

 function mergePositions(IERC20 collateralToken, Market market, uint256 amount) public {
        _mergePositions(collateralToken, market, amount);

 @       if (market.parentCollectionId() == bytes32(0)) {
            // send collateral tokens back to the user.
            collateralToken.transfer(msg.sender, amount);
        }
    }

in this case, if the parnetCollectionId() of a market should return 0 but not return 0, the collateral token is transferred out directly.

the fix is in MarketFactory

 function createMarket(
        CreateMarketParams memory params,
        string memory marketName,
        InternalMarketConfig memory config
    ) internal returns (address) {
        (Market.ConditionalTokensParams memory conditionalTokensParams, Market.RealityParams memory realityParams) =
            createNewMarketParams(params, config);

        Market instance = Market(market.clone());

        instance.initialize(
            marketName,
            params.outcomes,
            params.lowerBound,
            params.upperBound,
            conditionalTokensParams,
            realityParams,
            realityProxy
        );

        emit NewMarket(
            address(instance),
            marketName,
            params.parentMarket,
            conditionalTokensParams.conditionId,
            conditionalTokensParams.questionId,
            realityParams.questionsIds
        );

        markets.push(address(instance));

        // added code here +++++
        createdMarket[address(instance)] = true; 

        return address(instance);
    }

and

 function mergePositions(IERC20 collateralToken, Market market, uint256 amount) public {

        if(marketFactory.createdMarket[market] == false) {
            revert ("Invalid market"); // here
        }
        _mergePositions(collateralToken, market, amount);

 @       if (market.parentCollectionId() == bytes32(0)) {
            // send collateral tokens back to the user.
            collateralToken.transfer(msg.sender, amount);
        }
    }

Attachments

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

hats-finance / SeeR-PM-0x899bc13919880db76edf4ccd72bdfa5dfa666fb7

Market parameter is not validated in router when merge position #60