From a6d36923a1c05d4ddf088ba183195b497b5e5a0b Mon Sep 17 00:00:00 2001 From: Nick Cardin Date: Thu, 11 Jun 2026 17:27:04 -0400 Subject: [PATCH] fix: avoid panic on invalid signature input --- .../src/message_component/component_name.rs | 18 ++++++---- httpsig/src/signature_base.rs | 32 +++++++++++++++++ httpsig/src/signature_params.rs | 36 +++++++++++++------ 3 files changed, 70 insertions(+), 16 deletions(-) diff --git a/httpsig/src/message_component/component_name.rs b/httpsig/src/message_component/component_name.rs index 7164157..b3a1439 100644 --- a/httpsig/src/message_component/component_name.rs +++ b/httpsig/src/message_component/component_name.rs @@ -17,7 +17,7 @@ impl TryFrom<&BareItem> for HttpMessageComponentName { match value { BareItem::String(name) => { if name.as_str().starts_with('@') { - Ok(Self::Derived(DerivedComponentName::from(name.as_str()))) + Ok(Self::Derived(DerivedComponentName::try_from(name.as_str())?)) } else { Ok(Self::HttpField(name.to_string())) } @@ -75,9 +75,10 @@ impl From for String { val.as_ref().to_string() } } -impl From<&str> for DerivedComponentName { - fn from(val: &str) -> Self { - match val { +impl TryFrom<&str> for DerivedComponentName { + type Error = HttpSigError; + fn try_from(val: &str) -> HttpSigResult { + let component = match val { "@method" => Self::Method, "@target-uri" => Self::TargetUri, "@authority" => Self::Authority, @@ -88,8 +89,13 @@ impl From<&str> for DerivedComponentName { "@query-param" => Self::QueryParam, "@status" => Self::Status, "@signature-params" => Self::SignatureParams, - _ => panic!("Invalid derived component: {}", val), - } + _ => { + return Err(HttpSigError::InvalidComponentName(format!( + "Invalid derived component: {val}" + ))); + } + }; + Ok(component) } } diff --git a/httpsig/src/signature_base.rs b/httpsig/src/signature_base.rs index 4f07dd5..7b6b632 100644 --- a/httpsig/src/signature_base.rs +++ b/httpsig/src/signature_base.rs @@ -282,6 +282,38 @@ mod test { ); } + /// A hostile `Signature-Input` carrying a negative `created`/`expires` timestamp is a legal + /// SFV integer but not a valid unix timestamp. This should not panic. + #[test] + fn try_parse_rejects_negative_timestamp_without_panic() { + const SIGNATURE: &str = r##"sig1=:AAAA:"##; + + for hostile_input in [ + r##"sig1=("date");created=-1;keyid="x""##, + r##"sig1=("date");expires=-1;keyid="x""##, + ] { + let res = HttpSignatureHeaders::try_parse(SIGNATURE, hostile_input); + assert!( + matches!(res, Err(HttpSigError::InvalidSignatureParams(_))), + "expected InvalidSignatureParams, got {res:?}" + ); + } + } + + /// A hostile `Signature-Input` naming an unknown `@`-prefixed derived component should + /// not panic. + #[test] + fn try_parse_rejects_unknown_derived_component_without_panic() { + const SIGNATURE: &str = r##"sig1=:AAAA:"##; + const HOSTILE_INPUT: &str = r##"sig1=("@bogus");keyid="x""##; + + let res = HttpSignatureHeaders::try_parse(SIGNATURE, HOSTILE_INPUT); + assert!( + matches!(res, Err(HttpSigError::InvalidComponentName(_))), + "expected InvalidComponentName, got {res:?}" + ); + } + #[test] fn test_signature_values() { const SIGNATURE_INPUT: &str = r##"sig-b26=("date" "@method" "@path" "@authority" "content-type" "content-length");created=1618884473;keyid="test-key-ed25519", sig-b27=("date" "@method" "@path" "@authority" "content-type" "content-length");created=1618884473;keyid="test-key-ed25519-alt""##; diff --git a/httpsig/src/signature_params.rs b/httpsig/src/signature_params.rs index 122b66c..62e9102 100644 --- a/httpsig/src/signature_params.rs +++ b/httpsig/src/signature_params.rs @@ -189,17 +189,33 @@ impl TryFrom<&ListEntry> for HttpSignatureParams { inner_list_with_params .params .iter() - .for_each(|(key, bare_item)| match key.as_str() { - "created" => params.created = bare_item.as_integer().map(|v| v.try_into().unwrap()), - "expires" => params.expires = bare_item.as_integer().map(|v| v.try_into().unwrap()), - "nonce" => params.nonce = bare_item.as_string().map(|v| v.to_string()), - "alg" => params.alg = bare_item.as_string().map(|v| v.to_string()), - "keyid" => params.keyid = bare_item.as_string().map(|v| v.to_string()), - "tag" => params.tag = bare_item.as_string().map(|v| v.to_string()), - _ => { - error!("Ignore invalid signature parameter: {}", key) + .try_for_each(|(key, bare_item)| -> Result<(), HttpSigError> { + match key.as_str() { + // `created`/`expires` are unix timestamps. They must be non-negative integers;. + "created" => { + params.created = bare_item + .as_integer() + .map(u64::try_from) + .transpose() + .map_err(|_| HttpSigError::InvalidSignatureParams("`created` must be a non-negative integer".to_string()))?; + } + "expires" => { + params.expires = bare_item + .as_integer() + .map(u64::try_from) + .transpose() + .map_err(|_| HttpSigError::InvalidSignatureParams("`expires` must be a non-negative integer".to_string()))?; + } + "nonce" => params.nonce = bare_item.as_string().map(|v| v.to_string()), + "alg" => params.alg = bare_item.as_string().map(|v| v.to_string()), + "keyid" => params.keyid = bare_item.as_string().map(|v| v.to_string()), + "tag" => params.tag = bare_item.as_string().map(|v| v.to_string()), + _ => { + error!("Ignore invalid signature parameter: {}", key) + } } - }); + Ok(()) + })?; Ok(params) } }