diff --git a/der/src/error.rs b/der/src/error.rs index e9f914695..9acf950fb 100644 --- a/der/src/error.rs +++ b/der/src/error.rs @@ -232,7 +232,7 @@ pub enum ErrorKind { #[cfg(feature = "std")] Io(std::io::ErrorKind), - /// Indefinite length disallowed (or malformed when decoding BER) + /// Indefinite length invalid (disallowed in DER or malformed when decoding BER) IndefiniteLength, /// Incorrect length for a given field. @@ -241,6 +241,9 @@ pub enum ErrorKind { tag: Tag, }, + /// Too many nested productions (i.e. TLV-in-TLV) encountered. + NestingDepth, + /// Message is not canonically encoded. Noncanonical { /// Tag of the value which is not canonically encoded. @@ -373,6 +376,7 @@ impl fmt::Display for ErrorKind { ErrorKind::Io(err) => write!(f, "I/O error: {err:?}"), ErrorKind::IndefiniteLength => write!(f, "indefinite length disallowed/malformed"), ErrorKind::Length { tag } => write!(f, "incorrect length for {tag}"), + ErrorKind::NestingDepth => write!(f, "too many nested productions encountered"), ErrorKind::Noncanonical { tag } => { write!(f, "ASN.1 {tag} not canonically encoded as DER") } diff --git a/der/src/reader/position.rs b/der/src/reader/position.rs index 46388389d..0d160a291 100644 --- a/der/src/reader/position.rs +++ b/der/src/reader/position.rs @@ -10,14 +10,21 @@ pub(super) struct Position { /// Position in the input buffer (in bytes after Base64 decoding). position: Length, + + /// Number of nested productions encountered (i.e. depth in the call stack). + depth: usize, } 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 { input_len, position: Length::ZERO, + depth: 0, } } @@ -44,6 +51,11 @@ impl Position { self.position } + /// Create new [`Error`] from the given [`ErrorKind`] which includes the current position. + pub(super) fn error(&mut self, kind: ErrorKind) -> Error { + kind.at(self.position) + } + /// Get the input length. pub(super) fn input_len(&self) -> Length { self.input_len @@ -61,6 +73,11 @@ 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, + _ => return Err(self.error(ErrorKind::NestingDepth)), + } + let nested_input_len = (self.position + len)?; if nested_input_len > self.input_len { @@ -77,6 +94,7 @@ impl Position { /// Resume processing the rest of a message after processing a nested inner portion. pub(super) fn resume_nested(&mut self, resumption: Resumption) { self.input_len = resumption.input_len; + self.depth = self.depth.saturating_sub(1); } } diff --git a/der/src/reader/slice.rs b/der/src/reader/slice.rs index e4462a42a..4e5a7c4d5 100644 --- a/der/src/reader/slice.rs +++ b/der/src/reader/slice.rs @@ -48,7 +48,7 @@ impl<'a> SliceReader<'a> { /// error occurred. pub fn error(&mut self, kind: ErrorKind) -> Error { self.failed = true; - kind.at(self.position.current()) + self.position.error(kind) } /// Did the decoding operation fail due to an error? diff --git a/der/tests/nesting.rs b/der/tests/nesting.rs new file mode 100644 index 000000000..d8e81e703 --- /dev/null +++ b/der/tests/nesting.rs @@ -0,0 +1,50 @@ +//! Tests for handling of nested messages. + +use der::{Decode, Error, ErrorKind, Header, Reader, SliceReader, Tag, asn1::AnyRef}; + +// This is the expected maximum depth. +// TODO(tarcieri): expose this as a constant in the public API? +const MAX_DEPTH: usize = 64; + +fn walk<'a>(reader: &mut SliceReader<'a>) -> Result, Error> { + let header = Header::peek(reader)?; + if header.tag() == Tag::Sequence { + reader.sequence(|r| walk(r)) // der::read_nested recursion + } else { + AnyRef::decode(reader) + } +} + +#[test] +#[allow(clippy::cast_possible_truncation, reason = "test")] +fn returns_nesting_depth_error_when_max_depth_encountered() { + let depth = MAX_DEPTH + 1; + + let mut buf = vec![0x05, 0x00]; // innermost NULL + for _ in 0..depth { + let len = buf.len(); + let mut next = Vec::with_capacity(len + 5); + next.push(0x30); + if len < 0x80 { + next.push(len as u8); + } else if len <= 0xFF { + next.push(0x81); + next.push(len as u8); + } else if len <= 0xFFFF { + next.push(0x82); + next.push((len >> 8) as u8); + next.push(len as u8); + } else { + next.push(0x83); + next.push((len >> 16) as u8); + next.push((len >> 8) as u8); + next.push(len as u8); + } + next.extend_from_slice(&buf); + buf = next; + } + + let mut reader = SliceReader::new(&buf).unwrap(); + let err = walk(&mut reader).expect_err("should return ErrorKind::NestingDepth"); + assert_eq!(err.kind(), ErrorKind::NestingDepth); +}