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

Protocol loss in case price is far away from stop loss price. #482

Closed
code423n4 opened this issue Dec 16, 2022 · 5 comments
Closed

Protocol loss in case price is far away from stop loss price. #482

code423n4 opened this issue Dec 16, 2022 · 5 comments
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working duplicate-622 nullified Issue is high quality, but not accepted

Comments

@code423n4
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2022-12-tigris/blob/588c84b7bb354d20cbca6034544c4faa46e6a80e/contracts/TradingExtension.sol#L118

Vulnerability details

Impact

Each position when opened on Tigris has an optional stop loss price. Whenever market price drop below stop loss price, the position can be closed automatically by bot. However, there is a flaw in term of what price position should be closed. In the current codebase, even when market price drops below stop loss price, the position is still closed at stop loss price.

function _limitClose(
    uint _id,
    bool _tp,
    PriceData calldata _priceData,
    bytes calldata _signature
) external view returns(uint _limitPrice, address _tigAsset) {
    ...
    if (_tp) {
        ...
    } else {
        if (_trade.slPrice == 0) revert("7"); //LimitNotSet
        if (_trade.direction) {
            if (_trade.slPrice < _price) revert("6"); //LimitNotMet
        } else {
            if (_trade.slPrice > _price) revert("6"); //LimitNotMet
        }
        _limitPrice = _trade.slPrice; 
        // @audit High: Protocol loss in case price far from slPrice. 
        // e.g Long sl = 100, currentPrice = 90, should be close at 90
    }
}

If the market price is drop far away from stop loss price, protocol will result in loss.

Proof of Concept

Consider the scenario

  1. Alice has a long position, she set slPrice = 100.
  2. When market price is drop to 90, her position is still closed at 100 when it should be closed at 90. So she got 100 - 90 in price diff and this profit is taken from protocol.

Tools Used

Manual Review

Recommended Mitigation Steps

Consider closing position at market price when reaching stop loss. In addition to that, adding check to make sure close price is not too far, for example: more than 1%

@code423n4 code423n4 added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels Dec 16, 2022
code423n4 added a commit that referenced this issue Dec 16, 2022
@c4-judge
Copy link
Contributor

GalloDaSballo marked the issue as duplicate of #515

@c4-judge
Copy link
Contributor

GalloDaSballo marked the issue as duplicate of #622

@c4-judge
Copy link
Contributor

GalloDaSballo marked the issue as satisfactory

@c4-judge c4-judge added the satisfactory satisfies C4 submission criteria; eligible for awards label Jan 22, 2023
@GalloDaSballo
Copy link

Invalidating due to Warden being in breach of the Rules

@c4-judge
Copy link
Contributor

GalloDaSballo marked the issue as nullified

@c4-judge c4-judge added nullified Issue is high quality, but not accepted and removed satisfactory satisfies C4 submission criteria; eligible for awards labels Jan 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working duplicate-622 nullified Issue is high quality, but not accepted
Projects
None yet
Development

No branches or pull requests

3 participants