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

off-chain orders will not be able to be executed when shouldIncreaseNonce = true

SBSecurity

Summary

fillOffchainOrders() will always fail when multiple orders are submitted for a trading account.

Vulnerability Details

fillOffchainOrders() checks if the nonce signed by the trader is still the same in his trading account.

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

He can also specify if he wants to increase this nonce with shouldIncreaseNonce and if so, the nonce in his trading account will be increased, but all other off-chain orders for that account will fail because the nonce will no longer be the same and the above the check will revert.

This is a valid scenario because if the user has positions in 2 markets and wants to create an order for one of them, he will sign with the last nonce from his trading account. Then, while those orders are still not filled, he decides to create another order for the second market, he will again sign the same nonce because he cannot be sure which will be settled first. And as described, no matter which one is settled first, the second will fail. This will be even more frustrating in a real example, as fillOffchainOrders will wait and fill a batch of orders, not 1-2.

Impact

Users will have to sign new orders off-chain and miss the market state as it was when they signed the original order.

Tools Used

Manual Review

Recommendations

It is difficult to give a proper recommendation here, but a possible one is to compare the original nonce with the signed one, regardless of whether it increments an order, since the orders were valid before the nonce was incremented by another order and should still be able to be executed**.**

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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