Zaros Part 1
Zaros
DeFiFoundry
60,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Valid

A Malicious users can DOS `fillOffchainOrders` by submitting a valid offchain order and then increasing their nouce to cancel offchain order.

joshuajee

Summary

The fillOffchainOrders is called by the keeper with an array of valid off-chain orders belonging to different accounts, the problem here is that a trader can cancel an order before it is sent on-chain. Doing this will make the call to the fillOffchainOrders revert. Therefore the other users won't have their off-chain orders fulfilled.

Vulnerability Details

Offchain orders are sent on-chain by the keeper, the fillOffchainOrders loops through the array of orders provided by the keeper, and checks if they are valid, one of the checks here is that the off-chain order nonce is the same as the tradingAccount nonce. i.e (ctx.offchainOrder.nonce != tradingAccount.nonce), if the two nonces are not equal the transaction reverts.

https://github.com/Cyfrin/2024-07-zaros/blob/main/src/perpetuals/branches/SettlementBranch.sol#L236

function fillOffchainOrders(
uint128 marketId,
OffchainOrder.Data[] calldata offchainOrders,
bytes calldata priceData
)
external
onlyOffchainOrdersKeeper(marketId)
{
// working data
FillOffchainOrders_Context memory ctx;
...
for (uint256 i; i < offchainOrders.length; i++) {
ctx.offchainOrder = offchainOrders[i];
...
// load existing trading account; reverts for non-existent account
TradingAccount.Data storage tradingAccount =
TradingAccount.loadExisting(ctx.offchainOrder.tradingAccountId);
...
@-> if (ctx.offchainOrder.nonce != tradingAccount.nonce) {
@-> revert Errors.InvalidSignedNonce(tradingAccount.nonce, ctx.offchainOrder.nonce);
@-> }
...
}
​
}

Off-chain orders can be canceled by calling the cancelAllOffchainOrders function, this function can be called anytime by the trading account owner. This function increases the nonce and makes (ctx.offchainOrder.nonce != tradingAccount.nonce) it be true. Thereby causing a revert on the fillOffchainOrders function.

https://github.com/Cyfrin/2024-07-zaros/blob/main/src/perpetuals/branches/OrderBranch.sol#L374

function cancelAllOffchainOrders(uint128 tradingAccountId) external {
// load existing trading account; reverts for non-existent account
// enforces `msg.sender == owner` so only account owner can cancel
// pendin orders.
TradingAccount.Data storage tradingAccount =
TradingAccount.loadExistingAccountAndVerifySender(tradingAccountId);
​
// bump the nonce value, which will invalidate all offchain orders signed with the previous nonce
unchecked {
@-> tradingAccount.nonce++;
}
​
emit LogCancelAllOffchainOrders(msg.sender, tradingAccountId, tradingAccount.nonce);
}

POC

  1. Keeper is sending a list of 100 off-chain orders

  2. Bob a malicious user can cancel their offchain order just before the keeper sends it. This cancels the transaction for everyone.

Impact

Denial of service and Griefing.

Tools Used

Manual

Recommendations

Put the checks at the top of the function before any state changes and use of continue instead of revert.

function fillOffchainOrders(
uint128 marketId,
OffchainOrder.Data[] calldata offchainOrders,
bytes calldata priceData
)
external
onlyOffchainOrdersKeeper(marketId)
{
// working data
FillOffchainOrders_Context memory ctx;
...
for (uint256 i; i < offchainOrders.length; i++) {
ctx.offchainOrder = offchainOrders[i];
...
// load existing trading account; reverts for non-existent account
TradingAccount.Data storage tradingAccount =
TradingAccount.loadExisting(ctx.offchainOrder.tradingAccountId);
...
if (ctx.offchainOrder.nonce != tradingAccount.nonce) {
- revert Errors.InvalidSignedNonce(tradingAccount.nonce, ctx.offchainOrder.nonce);
+ continue;
}
...
}
​
}
Updates

Lead Judging Commences

inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Validated
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.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!