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

stent - Negative prices will cause old orders to be canceled (same as previous audit) #209

Closed
sherlock-admin opened this issue Jun 5, 2023 · 0 comments
Labels
Excluded Excluded by the judge without consulting the protocol or the senior Non-Reward This issue will not receive a payout

Comments

@sherlock-admin
Copy link
Contributor

stent

medium

Negative prices will cause old orders to be canceled (same as previous audit)

Summary

This was an issue from the previous Sherlock audit: sherlock-audit/2023-02-gmx-judging#177

In the above issue it was stated "Although unlikely the possibility of this must be addressed instead of canceling it" which is not case in the new code. A new error was added and thrown correctly as suggested, but not caught in the _handleOrderError function and so the order is still cancelled.

Vulnerability Detail

Same as previous audit.

Impact

Same as previous audit.

Code Snippet

Negative price causes custom error revert:

// File: gmx-synthetics/contracts/oracle/Oracle.sol : Oracle._setPricesFromPriceFeeds()   #1

        if (_price <= 0) {
            revert Errors.InvalidFeedPrice(token, _price);
        }

https://github.com/gmx-io/gmx-synthetics/blob/a2e331f6d0a3b59aaac5ead975b206840369a723/contracts/oracle/Oracle.sol#L604

Tool used

Manual Review

Recommendation

Add new error to the list of oracle errors in _handleOrderError that result in the tx being reverted with ErrorUtils.revertWithCustomError

@github-actions github-actions bot closed this as completed Jun 9, 2023
@github-actions github-actions bot added the Excluded Excluded by the judge without consulting the protocol or the senior label Jun 9, 2023
@sherlock-admin sherlock-admin added the Non-Reward This issue will not receive a payout label Jun 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Excluded Excluded by the judge without consulting the protocol or the senior Non-Reward This issue will not receive a payout
Projects
None yet
Development

No branches or pull requests

1 participant