Stop loss higher than openPrice may cause fund loss
Description
The function openTrade is used to open a new market or limit trade. While creating a limit order, the openPrice could be set to any value. For a buy order, the value of stop loss should always be less than this openPrice due to the check t.sl < t.openPrice as shown below in openTrade:
function openTrade(
ITradingStorage.Trade calldata t,
IExecute.OpenLimitOrderType _type,
uint _slippageP,
bytes[] calldata priceUpdateData,
uint _executionFee // In USDC. Optional for Limit orders
) external payable onlyWhitelist whenNotPaused {
//...
require(t.tp == 0 || (t.buy ? t.tp > t.openPrice : t.tp < t.openPrice),
"WRONG_TP");
require(t.sl == 0 || (t.buy ? t.sl < t.openPrice : t.sl > t.openPrice),
"WRONG_SL");
storageT.transferUSDC(msg.sender, address(storageT),
t.positionSizeUSDC + _executionFee);
if (_type != IExecute.OpenLimitOrderType.MARKET) {
uint index = storageT.firstEmptyOpenLimitIndex(msg.sender,
t.pairIndex);
storageT.storeOpenLimitOrder(
ITradingStorage.OpenLimitOrder(
msg.sender,
t.pairIndex,
index,
t.positionSizeUSDC,
t.buy,
t.leverage,
t.tp,
t.sl,
t.openPrice,
t.openPrice,
block.number,
_executionFee
)
);
aggregator.executions().setOpenLimitOrderType(msg.sender,
t.pairIndex, index, _type);
emit OpenLimitPlaced(msg.sender, t.pairIndex, index,
block.timestamp, _executionFee);
} else //...
}When a limit order is created, the maxPrice and the minPrice are set to the openPrice provided by user. If the order type is MOMENTUM, the executeLimitOrder could be called without errors as the require check a.price <= o.maxPrice in the executeLimitOpenOrderCallback callback function would pass as shown below:
function executeLimitOpenOrderCallback(AggregatorAnswer memory a)
external override onlyPriceAggregator {
//...
if (pairsStored.pairGroupIndex(o.pairIndex) == 0) {
// crypto only
(, uint priceAfterImpact) = pairInfos.getTradePriceImpact(
_marketExecutionPrice(a.price, a.spreadP, o.buy),
o.pairIndex,
o.buy,
o.positionSize.mul(o.leverage)
);
a.price = priceAfterImpact;
} else {
a.price = _marketExecutionPrice(a.price, a.spreadP, o.buy);
}
if (
t == IExecute.OpenLimitOrderType.MARKET
? (a.price >= o.minPrice && a.price <= o.maxPrice)
: (
t == IExecute.OpenLimitOrderType.REVERSAL
? (o.buy ? a.price >= o.maxPrice : a.price <= o.minPrice)
: (o.buy ? a.price <= o.maxPrice : a.price >= o.minPrice)
) && _withinExposureLimits(o.trader, o.pairIndex,
o.positionSize.mul(o.leverage))
) {
ITradingStorage.Trade memory finalTrade = _registerTrade(
ITradingStorage.Trade(
o.trader,
o.pairIndex,
0,
0,
o.positionSize,
a.price, // current price
o.buy,
o.leverage,
o.tp,
o.sl, // large value
0
)
);Here, the a.price is the current price of the token, o.sl is still larger than this price, and the trade would be registered. As the stop loss is greater than the current price of the token, the stop loss could be triggered successfully. When the stop loss is triggered, the profitP is calculated in the callback function executeLimitCloseOrderCallback:
function executeLimitCloseOrderCallback(AggregatorAnswer memory a)
external override onlyPriceAggregator {
//...
v.price = aggregator.pairsStorage().guaranteedSlEnabled(t.pairIndex)
? o.orderType == ITradingStorage.LimitOrder.TP ? t.tp :
o.orderType == ITradingStorage.LimitOrder.SL
? t.sl
: a.price
: a.price;
v.profitP = _currentPercentProfit(t.openPrice, v.price, t.buy,
t.leverage);
v.posToken = t.initialPosToken;
v.posUSDC = t.initialPosToken;
if (o.orderType == ITradingStorage.LimitOrder.LIQ) {
uint liqPrice = pairInfos.getTradeLiquidationPrice(
t.trader,
t.pairIndex,
t.index,
t.openPrice,
t.buy,
v.posUSDC,
t.leverage
);
v.reward = (t.buy ? a.price <= liqPrice : a.price >= liqPrice) ?
(v.posToken * liqFeeP) / 100 : 0;
} else {
v.reward = (o.orderType == ITradingStorage.LimitOrder.TP &&
t.tp > 0 &&
(t.buy ? a.price >= t.tp : a.price <= t.tp)) ||
(o.orderType == ITradingStorage.LimitOrder.SL &&
t.sl > 0 &&
(t.buy ? a.price <= t.sl : a.price >= t.sl))
? (
v.posToken.mul(t.leverage) *
aggregator.pairsStorage().pairLimitOrderFeeP(t.pairIndex)
) / 100 / _PRECISION
: 0;
}
if (o.orderType == ITradingStorage.LimitOrder.LIQ && v.reward > 0) {
uint usdcSentToTrader = _unregisterTrade(
t,
v.profitP,
v.posUSDC,
v.reward,
(v.reward * (liqTotalFeeP - liqFeeP)) / liqFeeP,
i.lossProtection
);When the profitP is calculated, it will set the v.price to the t.sl, which was higher than the current price of the token. Due to the vulnerability ā as the t.sl could be set higher than the t.openPrice ā it would always return in positive net profit when this function is called. The stop loss could be set to a really large value, leading to the maximum profit (900%) as allowed by the protocol and fund loss.
Here is an example trade:
Start
Balance of trader before: 10000000000
Trader places a limit order with large openPrice and stop loss just below it. Here is an example trade:
trade.trader = _trader;
trade.pairIndex = _pairIndex;
trade.index = _index;
trade.initialPosToken = 0;
trade.positionSizeUSDC = _amount;
trade.openPrice = 100000e10;
trade.buy = true;
trade.leverage = 10e10;
trade.tp = 0;
trade.sl = 100000e10-1;
trade.timestamp = block.number;
When the open order is executed, the openPrice is changed to the current price + price impact:
OpenPrice for the long order 505252500000000
Stop loss for the long order 999999999999999
Stop loss is then immediately executed as the value of stop loss is greater than the current price.
Balance of trader after: 97911000000Impact
An attacker can create a malicious trade such that they always make 900% returns instantly. They could use this to drain the protocol.
Recommendations
After we discussed the issue with the Avantis team, they suggested removing the MOMENTUM order type, which prevents users from setting stop loss higher than the current price and prevents this issue.
Remediation
This issue has been acknowledged by Avantis Labs, Inc., and a fix was implemented in commit 2609b4c5ā.