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

Ch_301 - The owner won't be able to create two Market with the same token ,strike and different ERC20 for deposit #300

Closed
sherlock-admin opened this issue Mar 27, 2023 · 6 comments
Labels
Escalation Resolved This issue's escalations have been approved/rejected Low/Info A valid Low/Informational severity issue Non-Reward This issue will not receive a payout Sponsor Confirmed The sponsor acknowledged this issue is valid

Comments

@sherlock-admin
Copy link
Contributor

Ch_301

medium

The owner won't be able to create two Market with the same token ,strike and different ERC20 for deposit

Summary

One of the changes compared to V1.
deposit asset can be any erc20

Vulnerability Detail

On createNewCarouselMarket()
The owner can't create two markets for the same token and strike with different underlyingAsset

e.g. in case there is a Market with token == x ,striek == y and underlyingAsset == WETH.
The owner won't be able to create another Market with token == x ,striek == y and underlyingAsset == e.g.USDC

Impact

The owner won't be able to create two Market with the same token ,strike and different ERC20 for deposit

Code Snippet

Tool used

Manual Review

Recommendation

    function createNewCarouselMarket(
        CarouselMarketConfigurationCalldata memory _marketCalldata
    )
        external
        onlyOwner
        returns (
            address premium,
            address collateral,
            uint256 marketId
        )
    {
        if (!controllers[_marketCalldata.controller]) revert ControllerNotSet();
        if (_marketCalldata.token == address(0)) revert AddressZero();
        if (_marketCalldata.oracle == address(0)) revert AddressZero();
        if (_marketCalldata.underlyingAsset == address(0)) revert AddressZero();

        if (tokenToOracle[_marketCalldata.token] == address(0)) {
            tokenToOracle[_marketCalldata.token] = _marketCalldata.oracle;
        }
-    marketId = getMarketId(_marketCalldata.token, _marketCalldata.strike);
+    marketId = getMarketId(_marketCalldata.token, _marketCalldata.strike, _marketCalldata.underlyingAsset);
@3xHarry
Copy link

3xHarry commented Apr 5, 2023

this makes sense

@3xHarry 3xHarry added the Sponsor Confirmed The sponsor acknowledged this issue is valid label Apr 5, 2023
3xHarry added a commit to Y2K-Finance/Earthquake that referenced this issue Apr 10, 2023
@sherlock-admin sherlock-admin added the Reward A payout will be made for this issue label Apr 11, 2023
@3xHarry
Copy link

3xHarry commented Apr 11, 2023

fix PR: Y2K-Finance/Earthquake#135

@twicek
Copy link

twicek commented Apr 11, 2023

Escalate for 10 USDC

This issue will never lead to a loss of funds because it is doesn't allow for creating a market in the first place in a specific case. As such, I think it should be considered low severity.

Additionally, the documentation says:

  • User experience and design improvement issues: Issues that cause minor inconvenience to users where there is no material loss of funds are not considered valid. Funds are temporarily stuck and can be recovered by the administrator or owner. Also, if a submission is a design opinion/suggestion without any clear indications of loss of funds is not a valid issue.

Neither this issue nor the duplicates succeed to show any clear indications of loss of funds.

@sherlock-admin
Copy link
Contributor Author

Escalate for 10 USDC

This issue will never lead to a loss of funds because it is doesn't allow for creating a market in the first place in a specific case. As such, I think it should be considered low severity.

Additionally, the documentation says:

  • User experience and design improvement issues: Issues that cause minor inconvenience to users where there is no material loss of funds are not considered valid. Funds are temporarily stuck and can be recovered by the administrator or owner. Also, if a submission is a design opinion/suggestion without any clear indications of loss of funds is not a valid issue.

Neither this issue nor the duplicates succeed to show any clear indications of loss of funds.

You've created a valid escalation for 10 USDC!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

@sherlock-admin sherlock-admin added the Escalated This issue contains a pending escalation label Apr 11, 2023
@hrishibhat
Copy link

Escalation accepted

Valid low
Based on the above comments the impact of this issue is a valid low as there is no loss of funds or breaking of any core functionality.

@sherlock-admin
Copy link
Contributor Author

Escalation accepted

Valid low
Based on the above comments the impact of this issue is a valid low as there is no loss of funds or breaking of any core functionality.

This issue's escalations have been accepted!

Contestants' payouts and scores will be updated according to the changes made on this issue.

@sherlock-admin sherlock-admin added Escalation Resolved This issue's escalations have been approved/rejected and removed Escalated This issue contains a pending escalation labels Apr 26, 2023
@hrishibhat hrishibhat removed Medium A valid Medium severity issue Reward A payout will be made for this issue labels Apr 28, 2023
@sherlock-admin sherlock-admin added Low/Info A valid Low/Informational severity issue Non-Reward This issue will not receive a payout and removed Has Duplicates A valid issue with 1+ other issues describing the same vulnerability labels Apr 28, 2023
3xHarry added a commit to Y2K-Finance/Earthquake that referenced this issue May 10, 2023
3xHarry added a commit to Y2K-Finance/Earthquake that referenced this issue May 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Escalation Resolved This issue's escalations have been approved/rejected Low/Info A valid Low/Informational severity issue Non-Reward This issue will not receive a payout Sponsor Confirmed The sponsor acknowledged this issue is valid
Projects
None yet
Development

No branches or pull requests

4 participants