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

Users can DoS the filling of off-chain orders

samuraii77

Summary

Users can DoS the filling of off-chain orders

Vulnerability Details

Users can create an off-chain order that a keeper then uses and fills on-chain using SettlementBranch::fillOffchainOrders(). We have this line there:

if (ctx.offchainOrder.nonce != tradingAccount.nonce) {
revert Errors.InvalidSignedNonce(tradingAccount.nonce, ctx.offchainOrder.nonce);
}

Any user that is in the off-chain orders array provided by the keeper can call this function that will make the code above revert:

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);
}

This will disallow the keeper from filling the off-chain orders. The bigger issue is that it would be extremely hard for the keeper to determine if someone is malicious and is just looking to DoS the filling of orders so he can just exclude him from the array. This makes it so a malicious user could always end up on that array as long as he wants to and DoS the function whenever he wants.

Impact

Users can DoS the filling of off-chain orders

Tools Used

Manual Review

Recommendations

Use continue instead of reverting, also refactor the rest of the code to not revert but instead continue whenever the function would revert due to the function failling because of a particular user

Updates

Lead Judging Commences

inallhonesty Lead Judge 10 months 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.