From da36f906cf6afe890a1a1bb74024755f4ca17d22 Mon Sep 17 00:00:00 2001 From: Pi Lanningham Date: Mon, 6 May 2024 22:14:20 -0400 Subject: [PATCH] Check the signature against the whole execution This is actually really subtly important; we were checking that the *details* were signed, which excludes the validity range and the txRef; that would have allowed someone to repurpose one signature for a different order. It's important that the signature covers the whole execution, not just the details. --- lib/calculation/strategy.ak | 10 ++++++---- lib/types/order.ak | 2 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/lib/calculation/strategy.ak b/lib/calculation/strategy.ak index 2b26d2b..07d2e54 100644 --- a/lib/calculation/strategy.ak +++ b/lib/calculation/strategy.ak @@ -74,10 +74,10 @@ pub fn get_strategy( tx_valid_range: ValidityRange, withdrawals: Dict, auth: StrategyAuthorization, - strategy: SignedStrategyExecution, + sse: SignedStrategyExecution, ) -> Order { - let SignedStrategyExecution { strategy, signature } = strategy - let StrategyExecution { tx_ref, validity_range, details, .. } = strategy + let SignedStrategyExecution { execution, signature } = sse + let StrategyExecution { tx_ref, validity_range, details, .. } = execution // We check that the order_tx_ref from the strategy matches the order we're actually processing, // to prevent replay / cross-play attacks. expect order_tx_ref == tx_ref @@ -89,7 +89,9 @@ pub fn get_strategy( Signature { signer } -> { // And finally, use cbor.serialise and check that the signature is valid // TODO: is this at risk if cbor.serialise changes? is there a way for us to get the raw bytes of the data? - let strategy_bytes = cbor.serialise(details) + // NOTE: it's really important that the signature is for the *whole execution* here; otherwise + // you could replay that signature over some other strategy order with the same signing key + let strategy_bytes = cbor.serialise(execution) expect Some(signature) = signature expect verify_signature(signer, strategy_bytes, signature) details diff --git a/lib/types/order.ak b/lib/types/order.ak index d347442..5f06371 100644 --- a/lib/types/order.ak +++ b/lib/types/order.ak @@ -109,7 +109,7 @@ pub type StrategyExecution { /// A specific strategy execution, plus a signature of that data pub type SignedStrategyExecution { /// The details about how to execute the strategy for a given order - strategy: StrategyExecution, + execution: StrategyExecution, /// An ed25519 signature of the serialized `strategy` signature: Option, }