DeFiFoundry
60,000 USDC
View results
Submission Details
Severity: medium
Invalid

Off-chain orders validation may fail making this functionnality unreliable

Summary

Zaros introduced a way for traders to create orders by signing them off-chain on the frontend.

This functionnality allows users to interact with their positions using a sort of order limit and without spending gas.

These orders are later on filled by the keepers through the SettlementBranch::fillOffchainOrders() function that uses ECDSA.recover from Openzeppelin to recover the address that signed the order and compare it to the account on which the order will be executed to make sure it is legitimate.

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

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
)
);
// ...
ctx.signer = ECDSA.recover(
_hashTypedDataV4(ctx.structHash), ctx.offchainOrder.v, ctx.offchainOrder.r, ctx.offchainOrder.s
);
if (ctx.signer != tradingAccount.owner) {
revert Errors.InvalidOrderSigner(ctx.signer, tradingAccount.owner);
}

Vulnerability Details

In order to prevent signature malleability attacks, ECDSA.recover() from Openzeppelin implemented the following sets of requirements that can be found in the library implementation :

  • the s value must be in the lower half order (taken from the negative side of the curve)

  • the v value to be either 27 or 28

Depending on the library used by the frontend, these requirements might not be met which would make the signature to be verified invalid from Openzeppelin ECDSA.recover() perspective.

Impact

Legitimate signatures can fail during the verification process, discarding them while they should not, which can disadvantage traders on Zaros.

Tools Used

Manual review

Recommendations

Enforce or adapt the library used by the frontend to align with Openzeppelin's s and v requirements following their recommendations.

Updates

Lead Judging Commences

inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Out of scope

Support

FAQs

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

Give us feedback!