Skip to content
This repository has been archived by the owner on Oct 1, 2023. It is now read-only.

0xvj - Admins cannot be able to create a new market with same insurance token and strikePrice but a different underlyingAsset token #376

Closed
sherlock-admin opened this issue Mar 27, 2023 · 0 comments
Labels
Non-Reward This issue will not receive a payout

Comments

@sherlock-admin
Copy link
Contributor

sherlock-admin commented Mar 27, 2023

0xvj

medium

Admins cannot be able to create a new market with same insurance token and strikePrice but a different underlyingAsset token

Summary

Admins cannot be able to create a new market with same token and strikePrice but a different underlyingAsset token as the marketId of a new market is being calculated using just token address and strikePrice as inputs.

Vulnerability Detail

The VaultFactoryV2.createNewMarket function creates a new market by calculating a marketId using the getMarketId function, which takes the token address and strike price as inputs. However, if an admin attempts to create a new market with the same token and strike price but a different underlying asset token, the VaultFactoryV2.createNewMarket function will fail and revert. This is because the new market would also receive the same marketId, causing a conflict.

To clarify, when theVaultFactoryV2.createNewMarket function is used to create a new market, it first calls the getMarketId function, which takes the token address and strike price as inputs and returns a unique market identifier. However, since the getMarketId function only takes these two parameters into account, it is not able to differentiate between markets with the same token and strike price but different underlying asset tokens. As a result, attempting to create a new market with these parameters will cause a conflict, leading to the function reverting.

Steps:

  1. Admin created a market in below configuration
      underlyingAsset = WETH
      token = USDC
      strikePrice = 0.9875
    
  2. Later all the user of the system wants to deposit stETH as collateral and premium instead of WETH.
  3. So now admin wants to create another market with below which accepts stETH as asset
     underlyingAsset = stETH
     token = USDC
     strikePrice = 0.9875
    
  4. createNewMarket function will revert while creating the new market with stETH as both markets have same token and stike price because of the below check.
    if (marketIdToVaults[marketId][0] != address(0))
                revert MarketAlreadyExists();

Impact

The inability to create a new market with a different underlying asset token may render the protocol unusable in the future if users prefer to use a different token as the underlying asset, particularly if that token is more popular and widely owned by most users(ex: stETH).

Code Snippet

// marketId is calculated using token address and strikePrice
 function getMarketId(address token, uint256 strikePrice)
        public
        pure
        returns (uint256 marketId)
    {
        return uint256(keccak256(abi.encodePacked(token, strikePrice)));
    }

https://github.com/Y2K-Finance/Earthquake/blob/736b2e1e51bef6daa6a5ecd1decb7d156316d795/src/v2/VaultFactoryV2.sol#L375-L381

function createNewMarket(MarketConfigurationCalldata memory _marketCalldata)
        external
        onlyOwner
        returns (
            address premium,
            address collateral,
            uint256 marketId
        )
    {
        
        //  ...

        marketId = getMarketId(_marketCalldata.token, _marketCalldata.strike);
        if (marketIdToVaults[marketId][0] != address(0))
            revert MarketAlreadyExists();

       // ...
       
        marketIdToVaults[marketId] = [premium, collateral];

        // ...

        return (premium, collateral, marketId);
    }

https://github.com/Y2K-Finance/Earthquake/blob/736b2e1e51bef6daa6a5ecd1decb7d156316d795/src/v2/VaultFactoryV2.sol#L58-L128

Tool used

Manual Review

Recommendation

  • Consider calculating the marketId from _marketCalldata.underlyingAsset _marketCalldata.token and _marketCalldata.strikePrice instead of calculating it from _marketCalldata.underlyingAsset and _marketCalldata.token in VaultFactoryV2.createNewMarket function.

    function createNewMarket(MarketConfigurationCalldata memory _marketCalldata)
            external
            onlyOwner
            returns (
                address premium,
                address collateral,
                uint256 marketId
            )
        {
            
            //  ...
    
       -   marketId = getMarketId(_marketCalldata.token, _marketCalldata.strike);
       +   marketId = getMarketId(_marketCalldata.underlyingAsset,_marketCalldata.token, _marketCalldata.strike);
            if (marketIdToVaults[marketId][0] != address(0))
                revert MarketAlreadyExists();
    
           // ...
           
            marketIdToVaults[marketId] = [premium, collateral];
    
            // ...
    
            return (premium, collateral, marketId);
        }
  • Chane the getMarketId function as below.

    function getMarketId(address underlyingAsset,address token, uint256 strikePrice)
          public
          pure
          returns (uint256 marketId)
    {
          return uint256(keccak256(abi.encodePacked(underlyingAsset, token, strikePrice)));
    }

Duplicate of #300

@github-actions github-actions bot closed this as completed Apr 3, 2023
@github-actions github-actions bot added Medium A valid Medium severity issue Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label labels Apr 3, 2023
@sherlock-admin sherlock-admin added the Reward A payout will be made for this issue label Apr 11, 2023
@sherlock-admin sherlock-admin added Non-Reward This issue will not receive a payout and removed Medium A valid Medium severity issue Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Reward A payout will be made for this issue labels Apr 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Non-Reward This issue will not receive a payout
Projects
None yet
Development

No branches or pull requests

1 participant