code-423n4 / 2022-12-tigris-findings

8 stars 4 forks source link

Lack of slippage protection for trade #530

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2022-12-tigris/blob/588c84b7bb354d20cbca6034544c4faa46e6a80e/contracts/Trading.sol#L163-L210

Vulnerability details

Impact

Lack of slippage protection would hurt the user.

Let say, if user want to trade asset for some X value for Y asset, due to sudden collapse of market the reutrn from the trade would be trivial one.

user would suffer huge loss because of this.

Proof of Concept

   struct Trade {
    uint margin;
    uint leverage;
    uint asset;
    bool direction;
    uint price;
    uint tpPrice;
    uint slPrice;
    uint orderType;
    address trader;
    uint id;
    address tigAsset;
    int accInterest;
}

struct MintTrade {
    address account;
    uint256 margin;
    uint256 leverage;
    uint256 asset;
    bool direction;
    uint256 price;
    uint256 tp;
    uint256 sl;
    uint256 orderType;
    address tigAsset;
}

slippage protection is very crucial parameter for DEX/DEFI.

There are no parameters to handle the slippage protection.

I had discussion with team (GainsGoblin#0001) , they told that they have not considered about it. But they told that it is good to consider

Not a bad idea to add this as a final check though

Tools Used

Recommended Mitigation Steps

Include the slippage parameter for trade and use it for trading.

This would ensure that user would assure the expected expected amount of returns.

GalloDaSballo commented 1 year ago

Unclear if this can be considered a vulnerability Also not super happy with the amount of details

TriHaz commented 1 year ago

We offer guaranteed prices, so the trade will be opened with the price the user saw on frontend, so a sudden collapse of market after initiating a trade wouldn't change it's price.

c4-sponsor commented 1 year ago

TriHaz marked the issue as sponsor disputed

GalloDaSballo commented 1 year ago

Because the system offers a fixed price, I believe the finding to be invalid

c4-judge commented 1 year ago

GalloDaSballo marked the issue as unsatisfactory: Invalid