2024-4-19 08:0:0 Author: securityboulevard.com(查看原文) 阅读量:6 收藏
Summary
During a recent exploration of a fork of Gains Network, we discovered two critical issues that might lead to loss of funds from the liquidity pool. These issues allow users to create trades such that they make 900% profit from every trade no matter the price of the token after the trade has been initalized. While the current version of Gains does not contain these issues, there are numerous forks of the Gains project still using the previous versions that might contain these issues. While the first issue wasn’t originally present in the Gains Network and was only introduced in the fork we audited, the second issue in this blog was present in the previous version of Gains Network.
In this article, we’ll go over these issues and investigate their potential implications.
The Gains Fork Under the Hood
Before digging into the exact details of these issues, let’s first understand how Gains works under the hood.
Gains is a decentralized leverage-trading platform. A leverage-trading platform allows users to use a smaller amount of initial funds to gain exposure to a larger trade position in an underlying asset. Essentially, users can borrow funds from the liquidity pool to place larger trades. This could potentially increase their gains (or losses) by multiple times.
The ABCs of Leverage Trading
Let’s first get familiar with a few concepts used in leverage trading.
Collateral — The amount of assets that traders are willing to commit to a trade.
Leverage — Leverage enables traders to amplify potential returns (or losses) by borrowing funds from the LiquidityPool. For instance, consider a trader with $1,000 who wants to go long on BTC. Without leverage, they would only profit from their $1,000 investment. However, using 10x leverage, they could transform their $1,000 investment into a $10,000 position. This means that any price movement of BTC will have 10 times the impact on their trade.
Stop Loss — A stop-loss (SL) order is a special type of order used to limit potential losses on an open position. For example, setting a stop-loss 10% below the current price for a long position will close the trade if the price drops below 10%, thus limiting losses.
Take Profit — Take-profit (TP) orders allow traders to automatically close a trade when a certain level of profit is reached. For instance, setting a take-profit order 20% above the current price for a long position will close the trade once the price reaches this threshold. This feature enables traders to secure profits from their trades.
Orders and Trades in Gains
With these concepts in mind, we can dive into the user process of placing orders and trades in the Gains fork.
Users can place three types of orders whether it is buy(long) or sell(short):
MARKET — Used when traders want to open the trade immediately. The trade is opened at the market price plus spread.
LIMIT — Used when traders want to go long at a lower price than present or go short if the price reaches a higher price than present.
STOP LIMIT — Used when traders want to go long if the price reaches a higher price than present or go short if the price reaches a lower price than present.
The Gains fork allows users to place trades using the function openTrade, which takes the following arguments,
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 {
where the Trade struct is defined as follows
struct Trade{
address trader;
uint pairIndex;
uint index;
uint initialPosToken;
uint positionSizeUSDC;
uint openPrice;
bool buy;
uint leverage;
uint tp;
uint sl;
uint timestamp;
}
and the OpenLimitOrderType enum is defined as shown below:
enum OpenLimitOrderType {
MARKET,
REVERSAL, // Stop Limit
MOMENTUM // Limit
}
A user can open a MARKET, REVERSAL, or MOMENTUM type of order. The MARKET order is fullfilled in the same transaction, whereas the other two orders are stored as pending limit orders with the openPrice set as the minPrice and maxPrice of the pending limit order. These pending limit orders could be fulfilled by anyone by calling the executeLimitOrder function, and in return they are paid the _executionFee set in the openTrade function.
The openTrade and executeLimitOrder functions call the fulfill function of the PriceAggregator, which verifies if the price is correct and then calls the appropriate function in the TradingCallback contract, which is responsible for opening and closing the trades.
While analyzing the Trade struct and the prices used to close the trade for different trade types, we stumbled upon the first critical issue.
Issue 1 — Stop Loss Higher Than openPrice
Now that we have gone through the inner workings of Gains, let’s start analyzing the first issue. As shown above, the Trade struct has a few different fields, including tp and sl, which refer to the take-profit and the stop-loss price that users want to set for a trade. When these values are set, trades could be closed with order types LimitOrder.TP and LimitOrder.SL if the respective price thresholds are met.
For example, let’s assume a trade was opened at an openPrice of $1,000 on the BTC/USD pair and the stop loss is set to $900. If the price of BTC with respect to USD reaches $900, the trade could be closed as the LimitOrder.SL order type. To close this trade, any user could call executeLimitOrder with the following parameters,
function executeLimitOrder(
ITradingStorage.LimitOrder _orderType,
address _trader, // The address of the trader
uint _pairIndex, // The index of the trading pair
uint _index, // The index of the order
bytes[] calldata priceUpdateData // Pyth price update data
) external payable onlyWhitelist whenNotPaused {
where _orderType could be one of these:
enum LimitOrder{
TP,
SL,
LIQ,
OPEN
}
After verifying if the liqPrice (liquidation price) is not above the sl for the buy(long) order — and vise versa for a sell(short) order — the function calls fulfill in the PriceAggregator contract to verify that the priceUpdateData is correct. The fulfill then calls executeLimitCloseOrderCallback in the TradingCallback contract to close the trade. The executeLimitCloseOrderCallback function calculates the profit percentage (positive for gains and negative for losses) of the trade, deducts some fees, transfers the remaining tokens to the trade, and unregisters the trade. The profit percentage is calculated as follows:
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);
//...
function _currentPercentProfit(
uint openPrice,
uint currentPrice,
bool buy,
uint leverage
) private pure returns (int p) {
int diff = buy ? (int(currentPrice) - int(openPrice)) : (int(openPrice) - int(currentPrice));
int minPnlP = int(_PRECISION) * (-100);
int maxPnlP = int(_MAX_GAIN_P) * int(_PRECISION);
p = (diff * 100 * int(_PRECISION.mul(leverage))) / int(openPrice);
p = p < minPnlP ? minPnlP : p > maxPnlP ? maxPnlP : p;
}
Assuming a trader opened a trade at the openPrice of $1,000 with 5x leverage and the SL was set to $900, if the price goes below $900, the _currentPercentProfit would return -50, indicating that the loss of the trade is 50% of the initial collateral deposited. In this calculation, the diff in _currentPercentProfit was calculated as the difference between currentPrice (which is t.sl in this case) and openPrice.
Here, we could ask a question: What if t.sl (or currentPrice) could be set greater than openPrice?
While analyzing the code, we asked ourselves the same question. Our initial assumption was that it wouldn’t be possible to do that for a buy order, but on further analysis we discovered a way to make such a trade, and this is exactly where the bug existed. If t.sl being greater than openPrice is possible, for a buy order, the value of diff would be positive, which would mean a net profit for the trader, even if the price of the token went down since the initial buy trade. There is a check in the openTrade function that prevents t.sl from being higher than t.openPrice:
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");
//...
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);
//...
This could be bypassed by setting t.openPrice just above t.sl, which would store the order as an open limit order and set the minPrice and maxPrice as t.openPrice. Next, if this order is opened with the type ITradingStorage.LimitOrder.MOMENTUM, the order could be fulfilled by calling executeLimitOrder. The order type of MOMENTUM is necessary as it bypasses the following check in the callback of executeLimitOrder:
function executeLimitOpenOrderCallback(AggregatorAnswer memory a) external override onlyPriceAggregator {
//...
IExecute.OpenLimitOrderType t = executor.openLimitOrderTypes(n.trader, n.pairIndex, n.index);
IPriceAggregator aggregator = storageT.priceAggregator();
IPairStorage pairsStored = aggregator.pairsStorage();
//...
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) // check
) && _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,
o.buy,
o.leverage,
o.tp,
o.sl,
0
)
);
The callback function would store this trade with openPrice as a.price (the current price of the token after the price impact). Doing so would open a trade with t.sl greater than t.openPrice, and consequently the trade could be closed for profit (max profit of 900% in this case). Here is an example of how the exploit works:
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: 97911000000
As shown above, the trader started with an inital balance of 10000000000 and placed a trade using this amount. The trade was placed such that stop loss was higher than openPrice when the trade was executed. Next, this trade was closed with type LimitOrder.SL, and the trade made an instant 900% profit on that trade (minus some trading fee). Hence, the issue allowed the trade to make an instant 900% profit on a trade, no matter how the price of the token changed after the trade was initialized.
Is This Issue in Your Fork?
There are two underlying issues that led to the vulnerability:
- When the order was opened, the position was using
a.priceas theopenPriceof the order for both limit order types. - The order could be triggered of type
SL, even if liquidation price is higher than stop loss for that order. This is because the fork allowed users to execute these orders, instead of NFT bots, which verify the order type before executing the order.
To check if the issue exists in your Gains fork, verify if a.price is used for both REVERSAL and MOMENTUM type of orders in the _registerTrade call of the executeLimitOpenOrderCallback function.
Remediation
If the protocol allows users to trigger such order, make sure that LIQ orders are prioritized over any order type; for example, make sure that LIQ order type is preferred over stop loss in the NFT bot.
Here is how Gains is currently using the correct price↗ as openPrice for different types of orders.
Issue 2 — Unsafe Cast to int
Let’s look again at the _currentPercentProfit function and focus specifically on the int conversions happening in that function:
function _currentPercentProfit(
uint openPrice,
uint currentPrice,
bool buy,
uint leverage
) private pure returns (int p) {
int diff = buy ? (int(currentPrice) - int(openPrice)) : (int(openPrice) - int(currentPrice));
int minPnlP = int(_PRECISION) * (-100);
int maxPnlP = int(_MAX_GAIN_P) * int(_PRECISION);
p = (diff * 100 * int(_PRECISION.mul(leverage))) / int(openPrice);
p = p < minPnlP ? minPnlP : p > maxPnlP ? maxPnlP : p;
}
The value of openPrice is set as a.price, which is the price of the token after price impact. This value can’t be a number greater than type(int256).max , so the type conversion is safe here, but what about currentPrice? As seen previously, the value of currentPrice comes from t.sl or t.tp, which could be set by the user. Therefore, these values could be set to very large integers that would convert to negative numbers when casted to int.
Let’s consider a sell order, with currentPrice as type(uint256).max. The resultant value of diff would be openPrice + 1 (int(type(uint256).max) = -1 ), and hence the profit percent would be nearly equal to 100 * leverage. Therefore, if the leverage is greater than 9, the function will return the profit as 900%.
As we need the value of currentPrice (t.tp or t.sl) to be type(uint256).max and the order to be a sell order, the only case where we could bypass the below check is when t.tp is set to type(uint256).max.
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.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;
}
//...
Take profit could be easily set to type(uint256).max using updateTp, which directly updates the value of t.tp without any additional checks:
function updateTp(uint _pairIndex, uint _index, uint _newTp) external onlyWhitelist whenNotPaused {
_updateTp(_pairIndex, _index, _newTp);
}
function _updateTp(uint _pairIndex, uint _index, uint _newTp) internal {
uint leverage = storageT.openTrades(msg.sender, _pairIndex, _index).leverage;
uint tpLastUpdated = storageT.openTradesInfo(msg.sender, _pairIndex, _index).tpLastUpdated;
require(leverage > 0, "NO_TRADE");
require(block.number - tpLastUpdated >= limitOrdersTimelock, "LIMIT_TIMELOCK");
storageT.updateTp(msg.sender, _pairIndex, _index, _newTp);
emit TpUpdated(msg.sender, _pairIndex, _index, _newTp, block.timestamp);
}
Here is an example of how the exploit works:
Start
Balance of trader before: 1000000000
Trader creates a short position:
trade.trader = _trader;
trade.pairIndex = _pairIndex;
trade.index = _index;
trade.initialPosToken = 0;
trade.positionSizeUSDC = _amount;
trade.openPrice = _price;
trade.buy = true;
trade.leverage = 10e10;
trade.tp = 0;
trade.sl = 0;
trade.timestamp = block.number;
Trader calls `updateTp` with the `_newTp` as `0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff`
When execute limit close is called of type `LimitOrder.TP`, the trader receives 900% profit.
Balance of trader after: 9791100000
Here, the trader started with 1000000000 and made 900% profit similar to the previous issue. Both of these issues are critical, as they allow trades to make 9x profit, no matter the price movement of the tokens. This could potentially allow attackers to steal all the funds from the LiquidityPool if there are no other security measures in place.
Is This Issue in Your Fork?
The underlying issue here is the casting of uint to int. If the updateTp function allows any uint256 as the TP price, and there are no other limit checks on TP in the executeLimitCloseOrderCallback function, the version of the Gains fork you’re using might be vulnerable to this bug.
Remediation
A quick way to fix the issue is to make sure that any number larger than type(int256).max is not allowed to be set as the TP price in the updateTp function. A similar check should be done for the updateSl function, if you do not implement a callback in the updateSl for range checks.
Gains Network’s Fix
These issues are mitigated by Gains Network, as they verify the value of sl and tp when the trade is closed. These values are verified to lie between the high and low of the price of the token since the last update block of sl or tp. If these values do not lie in that range, the current market price is used as the SL or TP price. Shown below is the relevant code:
v.price = o.orderType == IGNSTradingStorage.LimitOrder.TP
? t.tp
: (o.orderType == IGNSTradingStorage.LimitOrder.SL ? t.sl : v.liqPrice);
v.exactExecution = v.price > 0 && a.low <= v.price && a.high >= v.price;
if (v.exactExecution) {
v.reward1 = o.orderType == IGNSTradingStorage.LimitOrder.LIQ
? (v.posDai * 5) / 100
: (v.levPosDai * multiCollatDiamond.pairNftLimitOrderFeeP(t.pairIndex)) / 100 / PRECISION;
} else {
v.price = a.open;
Here, the value of v.exactExecution is true only if the sl or tp are within correct bounds, and if not, a.open is used as the price of sl or tp. Therefore, it is not possible to set an incorrect value to any of these variables, which essentially fixes the vulnerabilities.
Disclosure Timeline
- January 10, 2024 — Individually disclosed the issue to Gambit Trade↗.
- January 25, 2024 — Individually disclosed the issue to Holdstation Exchange↗.
- March 27, 2024 — Contacted SEAL (Crypto Security Alliance) to get in touch with additional projects.
- March 28, 2024 — Released an internal advisory for a few other Gains forks that used a previous version.
- April 5, 2024 — Included Krav Trade↗ in the internal advisory chat and disclosed the issue to them.
- April 19, 2024 — Coordinated release date.
Caution
We’ve identified two critical issues in the previous version of Gains that might lead to loss of funds from the protocol. Although we’ve reached out to all the teams we could find that are using the vulnerable version of Gains, there might be still some teams using the vulnerable version. If you are building on Gains and believe you are at risk from either of these two critical issues, reach out to us↗.
Acknowledgments
Thank you to Zellic engineers Vakzz, who identified the second issue, and Kuilin for their insight and valuable collaboration during our audit.
About Us
Zellic specializes in securing emerging technologies. Our security researchers have uncovered vulnerabilities in the most valuable targets, from Fortune 500s to DeFi giants.
Developers, founders, and investors trust our security assessments to ship quickly, confidently, and without critical vulnerabilities. With our background in real-world offensive security research, we find what others miss.
Contact us↗ for an audit that’s better than the rest. Real audits, not rubber stamps.
*** This is a Security Bloggers Network syndicated blog from Zellic — Research Blog authored by Zellic — Research Blog. Read the original post at: https://www.zellic.io/blog/finding-losses-in-gains
如有侵权请联系:admin#unsafe.sh