Skip to content

Commit

Permalink
Check the signature against the whole execution
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Quantumplation committed May 7, 2024
1 parent 6fe4535 commit da36f90
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 5 deletions.
10 changes: 6 additions & 4 deletions lib/calculation/strategy.ak
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,10 @@ pub fn get_strategy(
tx_valid_range: ValidityRange,
withdrawals: Dict<StakeCredential, Int>,
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
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion lib/types/order.ak
Original file line number Diff line number Diff line change
Expand Up @@ -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<Signature>,
}

0 comments on commit da36f90

Please sign in to comment.