Submission Details
Severity:
Potential DDOS Vulnerability in fillOffchainOrders Function
Summary
There is a potential Distributed Denial of Service (DDOS) vulnerability in the fillOffchainOrders function of the protocol. This is due to the number of conditions in the function that can lead to reverting the function.
Vulnerability Details
The fillOffchainOrders function is vulnerable to DDOS attacks due to its ability to process an unbounded number of offchain orders. Malicious actors could exploit this by submitting a large number of orders that will revert, causing it to become unresponsive. A malicious actor can add an order that has a size of zero, wrong marketId, invalid nonce, and others, any of the listed actions will cause the function to fail. This attack is very likely to occur because it would not cost the attacker anything.
function fillOffchainOrders(
uint128 marketId,
OffchainOrder.Data[] calldata offchainOrders,
bytes calldata priceData
)
external
onlyOffchainOrdersKeeper(marketId)
{
// working data
FillOffchainOrders_Context memory ctx;
// fetch storage slot for perp market's offchain order config
SettlementConfiguration.Data storage settlementConfiguration =
SettlementConfiguration.load(marketId, SettlementConfiguration.OFFCHAIN_ORDERS_CONFIGURATION_ID);
// fetch storage slot for perp market
PerpMarket.Data storage perpMarket = PerpMarket.load(marketId);
// fetch storage slot for global config
GlobalConfiguration.Data storage globalConfiguration = GlobalConfiguration.load();
// verifies provided price data following the configured settlement strategy
// returning the bid and ask prices
(ctx.bidX18, ctx.askX18) =
settlementConfiguration.verifyOffchainPrice(priceData, globalConfiguration.maxVerificationDelay);
// iterate through off-chain orders; intentionally not caching
// length as reading from calldata is faster
for (uint256 i; i < offchainOrders.length; i++) {
ctx.offchainOrder = offchainOrders[i];
// enforce size > 0
if (ctx.offchainOrder.sizeDelta == 0) {
@> revert Errors.ZeroInput("offchainOrder.sizeDelta");
}
// load existing trading account; reverts for non-existent account
TradingAccount.Data storage tradingAccount =
TradingAccount.loadExisting(ctx.offchainOrder.tradingAccountId);
// enforce that keeper is filling the order for the correct marketId
if (marketId != ctx.offchainOrder.marketId) {
@> revert Errors.OrderMarketIdMismatch(marketId, ctx.offchainOrder.marketId);
}
// First we check if the nonce is valid, as a first measure to protect from replay attacks, according to
// the offchain order's type (each type may have its own business logic).
// e.g TP/SL must increase the nonce in order to prevent older limit orders from being filled.
// NOTE: Since the nonce isn't always increased, we also need to store the typed data hash containing the
// 256-bit salt value to fully prevent replay attacks.
if (ctx.offchainOrder.nonce != tradingAccount.nonce) {
@> revert Errors.InvalidSignedNonce(tradingAccount.nonce, ctx.offchainOrder.nonce);
}
ctx.structHash = keccak256(
abi.encode(
Constants.CREATE_OFFCHAIN_ORDER_TYPEHASH,
ctx.offchainOrder.tradingAccountId,
ctx.offchainOrder.marketId,
ctx.offchainOrder.sizeDelta,
ctx.offchainOrder.targetPrice,
ctx.offchainOrder.shouldIncreaseNonce,
ctx.offchainOrder.nonce,
ctx.offchainOrder.salt
)
);
// If the offchain order has already been filled, revert.
// we store `ctx.hash`, and expect each order signed by the user to provide a unique salt so that filled
// orders can't be replayed regardless of the account's nonce.
if (tradingAccount.hasOffchainOrderBeenFilled[ctx.structHash]) {
@> revert Errors.OrderAlreadyFilled(ctx.offchainOrder.tradingAccountId, ctx.offchainOrder.salt);
}
// `ecrecover`s the order signer.
ctx.signer = ECDSA.recover(
_hashTypedDataV4(ctx.structHash), ctx.offchainOrder.v, ctx.offchainOrder.r, ctx.offchainOrder.s
);
// ensure the signer is the owner of the trading account, otherwise revert.
// NOTE: If an account's owner transfers to another address, this will fail. Therefore, clients must
// cancel all users offchain orders in that scenario.
if (ctx.signer != tradingAccount.owner) {
@> revert Errors.InvalidOrderSigner(ctx.signer, tradingAccount.owner);
}
// cache the order side
ctx.isBuyOrder = ctx.offchainOrder.sizeDelta > 0;
// buy order -> match against the ask price
// sell order -> match against the bid price
ctx.indexPriceX18 = ctx.isBuyOrder ? ctx.askX18 : ctx.bidX18;
// verify the provided price data against the verifier and ensure it's valid, then get the mark price
// based on the returned index price.
ctx.fillPriceX18 = perpMarket.getMarkPrice(sd59x18(ctx.offchainOrder.sizeDelta), ctx.indexPriceX18);
// if the order increases the trading account's position (buy order), the fill price must be less than or
// equal to the target price, if it decreases the trading account's position (sell order), the fill price
// must be greater than or equal to the target price.
ctx.isFillPriceValid = (ctx.isBuyOrder && ctx.offchainOrder.targetPrice <= ctx.fillPriceX18.intoUint256())
|| (!ctx.isBuyOrder && ctx.offchainOrder.targetPrice >= ctx.fillPriceX18.intoUint256());
// we don't revert here because we want to continue filling other orders.
if (!ctx.isFillPriceValid) {
continue;
}
// account state updates start here
// increase the trading account nonce if the order's flag is true.
if (ctx.offchainOrder.shouldIncreaseNonce) {
unchecked {
tradingAccount.nonce++;
}
}
// mark the offchain order as filled.
// we store the struct hash to be marked as filled.
tradingAccount.hasOffchainOrderBeenFilled[ctx.structHash] = true;
// fill the offchain order.
_fillOrder(
ctx.offchainOrder.tradingAccountId,
marketId,
SettlementConfiguration.OFFCHAIN_ORDERS_CONFIGURATION_ID,
sd59x18(ctx.offchainOrder.sizeDelta),
ctx.fillPriceX18
);
}
}
Impact
There will be denial of service on the fillOffchainOrders Function making it unusable for traders.
Traders will be unable to successfully create OffchainOrders
Tools Used
Manual Review
Recommendations
Instead of reverting, the continue statement should be used to skip the current order
Updates
Lead Judging Commences
inallhonesty about 1 year ago
Submission Judgement Published Assigned finding tags:
fillOffchainOrders reverts everything if a single order fails one of the multiple checks
If you send 1 cancel and 1 create it should still run the cancel, not revert everything.
Prize pool breakdown
Total prize
60,000 USDC
nSLOC
2,87821 USDC / LOC
50,000 USDC
5,500 USDC
4,500 USDC
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.