From 0522f9b2c4bb973331aca807dd532b8c6b5b52c4 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Tue, 15 Sep 2026 16:58:00 +0000 Subject: [PATCH 1/2] fix(x509): use checked length arithmetic in CertificateBundle::try_from CertificateBundle::try_from(&[u8]) parsed concatenated DER with unchecked arithmetic: total_len = header_len + cert_len and offset + total_len. An attacker-controlled long-form DER length can reach ~usize::MAX (parse_der_length folds up to 8 bytes with no width cap), making offset + total_len wrap below data.len() after one valid certificate advances offset. The bounds guard reuses the wrapped value and passes, after which &data[offset..offset+total_len] is a start>end slice that panics -> process/wasm abort under panic=abort. Reachable from the WASM export X509Certificate::parseChain on caller-supplied hex-DER. Use checked_add for both sums and bail on overflow; also reject long-form lengths wider than size_of::() in parse_der_length. Co-authored-by: Ty Schenk --- keetanetwork-x509/src/certificates.rs | 40 ++++++++++++++++++++++++--- keetanetwork-x509/src/utils.rs | 5 +++- 2 files changed, 40 insertions(+), 5 deletions(-) diff --git a/keetanetwork-x509/src/certificates.rs b/keetanetwork-x509/src/certificates.rs index b441e7b..6e8935a 100644 --- a/keetanetwork-x509/src/certificates.rs +++ b/keetanetwork-x509/src/certificates.rs @@ -681,15 +681,25 @@ impl TryFrom<&[u8]> for CertificateBundle { while offset < data.len() { // Parse DER length to get exact certificate size if let Some((cert_len, header_len)) = parse_der_length(&data[offset..]) { - let total_len = header_len + cert_len; + // Use checked arithmetic: an attacker-controlled long-form DER + // length can be close to `usize::MAX`, and unchecked addition + // would wrap `offset + total_len` below `data.len()`, letting a + // start > end slice pass the bounds check and panic (aborting + // the process under `panic = "abort"`). + let Some(total_len) = header_len.checked_add(cert_len) else { + break; + }; + let Some(end) = offset.checked_add(total_len) else { + break; + }; // Extract the complete certificate DER data - if offset + total_len <= data.len() { - let cert_data = &data[offset..offset + total_len]; + if end <= data.len() { + let cert_data = &data[offset..end]; if let Ok(cert) = Certificate::try_from(cert_data) { certificates.push(cert); - offset += total_len; + offset = end; } else { break; } @@ -2150,6 +2160,28 @@ mod tests { Ok(()) } + #[test] + fn test_bundle_try_from_rejects_overflow_length_without_panic() { + // Regression: a valid certificate followed by an element whose DER + // long-form length is close to usize::MAX must not overflow the length + // arithmetic into an out-of-range slice (which panics/aborts). The + // parser should stop cleanly and keep only the certificates it could + // safely parse. + test_all_certificate_sets(|bundle| { + let CertificateTestBundle { client_cert, .. } = bundle; + let mut data = client_cert.to_der()?; + // SEQUENCE tag, long-form 8-byte length = 0xFFFFFFFFFFFFFFF5 (~usize::MAX) + data.extend_from_slice(&[0x30, 0x88, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xF5]); + + // Must return (Ok or Err) without panicking; if Ok, only the leading + // valid certificate is retained. + if let Ok(parsed) = CertificateBundle::try_from(data.as_slice()) { + assert_eq!(parsed.into_iter().count(), 1); + } + Ok(()) + }); + } + #[test] fn test_certificate_with_options_bundle_functionality() -> Result<(), CertificateError> { /// Helper to test bundle roundtrip diff --git a/keetanetwork-x509/src/utils.rs b/keetanetwork-x509/src/utils.rs index b527611..162dda7 100644 --- a/keetanetwork-x509/src/utils.rs +++ b/keetanetwork-x509/src/utils.rs @@ -357,7 +357,10 @@ pub fn parse_der_length(data: impl AsRef<[u8]>) -> Option<(usize, usize)> { } else { // Long form: length is encoded in the following bytes let length_bytes = (length_byte & 0x7F) as usize; - if length_bytes == 0 || data.len() < 2 + length_bytes { + // Reject a length that cannot fit in `usize`; folding more than + // `size_of::()` bytes would silently shift out high bits and + // yield a bogus (wrapped) length. + if length_bytes == 0 || length_bytes > core::mem::size_of::() || data.len() < 2 + length_bytes { return None; } From 97ad53ed4d667543a04ff69695af7b59955b0744 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Tue, 15 Sep 2026 17:13:50 +0000 Subject: [PATCH 2/2] test(x509): propagate Result from test_all_certificate_sets (clippy -D warnings) The regression test discarded the Result returned by test_all_certificate_sets, tripping clippy's unused_must_use under -D warnings. Return it from the test so cargo clippy --all-targets --all-features -- -D warnings is clean. Co-authored-by: Ty Schenk --- keetanetwork-x509/src/certificates.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/keetanetwork-x509/src/certificates.rs b/keetanetwork-x509/src/certificates.rs index 6e8935a..d62ebe0 100644 --- a/keetanetwork-x509/src/certificates.rs +++ b/keetanetwork-x509/src/certificates.rs @@ -2161,7 +2161,7 @@ mod tests { } #[test] - fn test_bundle_try_from_rejects_overflow_length_without_panic() { + fn test_bundle_try_from_rejects_overflow_length_without_panic() -> Result<(), CertificateError> { // Regression: a valid certificate followed by an element whose DER // long-form length is close to usize::MAX must not overflow the length // arithmetic into an out-of-range slice (which panics/aborts). The @@ -2179,7 +2179,7 @@ mod tests { assert_eq!(parsed.into_iter().count(), 1); } Ok(()) - }); + }) } #[test]