diff --git a/der/src/length/indefinite.rs b/der/src/length/indefinite.rs index 511edcf3b..19fc767a2 100644 --- a/der/src/length/indefinite.rs +++ b/der/src/length/indefinite.rs @@ -19,10 +19,11 @@ //! > //! > The end-of-contents octets shall consist of two zero octets. -use crate::{Decode, ErrorKind, Header, Length, Reader}; +use super::INDEFINITE_LENGTH_OCTET; +use crate::{Decode, ErrorKind, Length, Reader, Tag, reader::MAX_NESTING_DEPTH}; #[cfg(feature = "alloc")] -use crate::Tag; +use crate::Header; #[cfg(feature = "alloc")] use alloc::vec::Vec; @@ -36,20 +37,49 @@ const EOC_TAG: u8 = 0x00; /// resulting length based on the amount of data decoded. pub(super) fn decode_indefinite_length<'a>(reader: &mut impl Reader<'a>) -> crate::Result { let start_pos = reader.position(); + let mut depth = 1; loop { // Look for the end-of-contents marker if reader.peek_byte() == Some(EOC_TAG) { read_eoc(reader)?; + depth -= 1; - // Compute how much we read and flag the decoded length as indefinite - let mut ret = (reader.position() - start_pos)?; - ret.indefinite = true; - return Ok(ret); + if depth == 0 { + // Compute how much we read and flag the decoded length as indefinite + let mut ret = (reader.position() - start_pos)?; + ret.indefinite = true; + return Ok(ret); + } + + continue; } - let header = Header::decode(reader)?; - reader.drain(header.length())?; + let (tag, is_constructed) = Tag::decode_with_constructed_bit(reader)?; + + if reader.peek_byte() == Some(INDEFINITE_LENGTH_OCTET) { + reader.read_byte()?; + + if !is_constructed { + return Err(reader.error(ErrorKind::IndefiniteLength)); + } + + if depth >= MAX_NESTING_DEPTH { + return Err(reader.error(ErrorKind::NestingDepth)); + } + + depth += 1; + } else { + // Only decode definite lengths here: indefinite lengths would recurse. + let length = Length::decode(reader).map_err(|e| { + if e.kind() == ErrorKind::Overlength { + reader.error(tag.length_error()) + } else { + e + } + })?; + reader.drain(length)?; + } } } diff --git a/der/src/reader.rs b/der/src/reader.rs index d5e1cd622..17d10750c 100644 --- a/der/src/reader.rs +++ b/der/src/reader.rs @@ -17,6 +17,9 @@ use alloc::vec::Vec; #[cfg(feature = "ber")] use crate::length::indefinite::read_eoc; +/// Maximum nesting depth for readers and BER indefinite-length scanning. +pub(crate) const MAX_NESTING_DEPTH: usize = 64; + /// Reader trait which reads DER-encoded input. pub trait Reader<'r>: Clone { /// Does this reader support the `read_slice` method? (i.e. can it borrow from his input?) diff --git a/der/src/reader/position.rs b/der/src/reader/position.rs index 673d7b1c6..9430383d5 100644 --- a/der/src/reader/position.rs +++ b/der/src/reader/position.rs @@ -1,5 +1,6 @@ //! Position tracking for processing nested input messages using only the stack. +use super::MAX_NESTING_DEPTH; use crate::{Error, ErrorKind, Length, Result}; /// State tracker for the current position in the input. @@ -16,9 +17,6 @@ pub(super) struct Position { } impl Position { - /// Maximum number of nested messages we tolerate (prevents stack exhaustion). - const MAX_DEPTH: usize = 64; - /// Create a new position tracker with the given overall length. pub(super) fn new(input_len: Length) -> Self { Self { @@ -74,7 +72,7 @@ impl Position { /// A [`Resumption`] value which can be used to continue parsing the outer message. pub(super) fn split_nested(&mut self, len: Length) -> Result { match self.depth.checked_add(1) { - Some(depth) if depth <= Self::MAX_DEPTH => self.depth = depth, + Some(depth) if depth <= MAX_NESTING_DEPTH => self.depth = depth, _ => return Err(self.error(ErrorKind::NestingDepth)), } diff --git a/der/tests/ber.rs b/der/tests/ber.rs new file mode 100644 index 000000000..1f2fa80ed --- /dev/null +++ b/der/tests/ber.rs @@ -0,0 +1,129 @@ +//! Regression tests for BER indefinite-length scanning. + +#![cfg(feature = "ber")] +#![allow(clippy::unwrap_used, reason = "tests")] + +use der::{ + Decode, EncodingRules, ErrorKind, Header, Length, Reader, SliceReader, Tag, asn1::AnyRef, +}; +use hex_literal::hex; + +/// Wrap a `NULL` in `depth` indefinite-length `SEQUENCE`s. +fn nested_sequences(depth: usize) -> Vec { + let mut bytes = Vec::with_capacity(4 * depth + 2); + for _ in 0..depth { + bytes.extend_from_slice(&[0x30, 0x80]); + } + bytes.extend_from_slice(&[0x05, 0x00]); + bytes.resize(4 * depth + 2, 0); + bytes +} + +fn assert_length(bytes: &[u8]) { + let mut reader = SliceReader::new_with_encoding_rules(bytes, EncodingRules::Ber).unwrap(); + let header = Header::decode(&mut reader).unwrap(); + assert_eq!(header.tag(), Tag::Sequence); + assert_eq!(usize::try_from(header.length()).unwrap(), bytes.len() - 2); + assert_eq!(reader.position(), Length::new(2)); + assert_eq!(AnyRef::from_ber(bytes).unwrap().value(), &bytes[2..]); +} + +#[test] +fn nested_indefinite_lengths() { + for depth in [1, 2, 64] { + assert_length(&nested_sequences(depth)); + } +} + +#[test] +fn indefinite_length_nesting_limit() { + let bytes = nested_sequences(65); + let err = AnyRef::from_ber(&bytes).unwrap_err(); + assert_eq!(err.kind(), ErrorKind::NestingDepth); +} + +#[test] +fn deeply_nested_indefinite_lengths() { + let bytes = nested_sequences(100_000); + let err = AnyRef::from_ber(&bytes).unwrap_err(); + assert_eq!(err.kind(), ErrorKind::NestingDepth); + + #[cfg(feature = "alloc")] + assert_eq!( + der::asn1::Any::from_ber(&bytes).unwrap_err().kind(), + ErrorKind::NestingDepth + ); +} + +#[test] +fn indefinite_length_siblings() { + let mut bytes = vec![0x30, 0x80]; + for _ in 0..100 { + bytes.extend_from_slice(&nested_sequences(1)); + } + bytes.extend_from_slice(&[0, 0]); + assert_length(&bytes); +} + +#[test] +fn mixed_definite_and_indefinite_lengths() { + // Definite-length contents, including apparent headers and EOCs, are opaque. + assert_length(&hex!( + "30 80 30 02 05 00 04 04 30 80 00 00 30 80 05 00 00 00 05 00 00 00" + )); + + // Constructed OCTET STRING: use the encoded constructed bit, not `Tag::is_constructed`. + assert_length(&hex!("30 80 24 80 04 02 00 00 00 00 00 00")); + + // A constructed context-specific tag in long form. + assert_length(&hex!("30 80 BF 81 00 80 05 00 00 00 00 00")); +} + +#[test] +fn primitive_indefinite_lengths() { + for bytes in [ + hex!("04 80 00 00").as_slice(), + &hex!("30 80 04 80 00 00 00 00"), + &hex!("30 80 9F 81 00 80 00 00 00 00"), + ] { + assert_eq!( + AnyRef::from_ber(bytes).unwrap_err().kind(), + ErrorKind::IndefiniteLength + ); + } +} + +#[test] +fn malformed_indefinite_lengths() { + for bytes in [ + hex!("30 80").as_slice(), + &hex!("30 80 00"), + &hex!("30 80 30 80 00 00"), + &hex!("30 80 04"), + &hex!("30 80 04 05 00 00"), + ] { + assert!(matches!( + AnyRef::from_ber(bytes).unwrap_err().kind(), + ErrorKind::Incomplete { .. } + )); + } + + assert_eq!( + AnyRef::from_ber(&hex!("30 80 00 01")).unwrap_err().kind(), + ErrorKind::IndefiniteLength + ); + assert_eq!( + AnyRef::from_ber(&hex!("30 80 04 85")).unwrap_err().kind(), + ErrorKind::Length { + tag: Tag::OctetString + } + ); +} + +#[test] +fn der_rejects_indefinite_lengths() { + assert_eq!( + AnyRef::from_der(&nested_sequences(1)).unwrap_err().kind(), + ErrorKind::IndefiniteLength + ); +}