diff --git a/CHANGELOG.md b/CHANGELOG.md index 0750cd8..07b5055 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,28 @@ ### Thanks +## 0.5.1 + +### Changed/Fixed + +Security: + +Fix a potential Denial-of-Service triggered by sending a specially +crafted packet containing a deeply recursive filter. +Parsing the filter can trigger a stack overflowm causing crash of +application using ldap-parser. +Affected functions are `Filter::from_ber` and functions calling it (like +`LdapMessage::from_ber`). + +Correction: filter parsing functions now include a compile-time fixed +maximum depth. Another parsing function has been added with a maximum +depth argument, for convenience. + +This was reported independently by multiple sources, credits go to: +- The Suricata Team +- Nozomi Networks Labs +- Kevin Valerio and Quan Nguyen from Trail of Bits in collaboration with OpenAI + ## 0.5.0 ### Changed/Fixed diff --git a/Cargo.lock b/Cargo.lock index 5d2a26c..da7a5d3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -65,7 +65,7 @@ checksum = "6fe2267d4ed49bc07b63801559be28c718ea06c4738b7a03c94df7386d2cde46" [[package]] name = "ldap-parser" -version = "0.5.0" +version = "0.5.1" dependencies = [ "asn1-rs", "hex-literal", diff --git a/Cargo.toml b/Cargo.toml index a2083ff..68d399a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "ldap-parser" -version = "0.5.0" +version = "0.5.1" description = "Parser for the LDAP protocol (RFC 4511)" authors = ["Pierre Chifflier "] edition = "2018" diff --git a/src/error.rs b/src/error.rs index 77db323..ac66276 100644 --- a/src/error.rs +++ b/src/error.rs @@ -32,6 +32,9 @@ pub enum LdapError { #[error("Invalid Type for Message")] InvalidMessageType, + #[error("Parsing Filter reached maximum depth")] + FilterMaxDepth, + #[error("Unknown error")] Unknown, @@ -76,7 +79,7 @@ pub(crate) fn print_hex_dump(bytes: &[u8], max_len: usize) { if m == 0 { println!(""); } - print!("{}", &bytes[..m].to_hex(16)); + print!("{}", bytes[..m].to_hex(16)); if bytes.len() > max_len { println!("... "); } diff --git a/src/filter_parser.rs b/src/filter_parser.rs index 9a2667d..0aebc3f 100644 --- a/src/filter_parser.rs +++ b/src/filter_parser.rs @@ -108,6 +108,9 @@ impl<'a> FromBer<'a, LdapError> for Attribute<'a> { // MatchingRuleId ::= LDAPString +/// Attempt to parse a `Filter` object and return the result, or an error +/// +/// This function is recursive, and has a maximum limit (see `MAX_FILTER_DEPTH` constant) // Filter ::= CHOICE { // and [0] SET SIZE (1..MAX) OF filter Filter, // or [1] SET SIZE (1..MAX) OF filter Filter, @@ -121,7 +124,31 @@ impl<'a> FromBer<'a, LdapError> for Attribute<'a> { // extensibleMatch [9] MatchingRuleAssertion, // ... } impl<'a> FromBer<'a, LdapError> for Filter<'a> { + #[inline] fn from_ber(bytes: &'a [u8]) -> ParseResult<'a, Self, LdapError> { + filter_from_ber(MAX_FILTER_DEPTH)(bytes) + } +} + +/// Helper function to build a combinator to parse a `Filter` parser, with depth limit argument +#[inline] +const fn filter_from_ber<'i>( + limit: usize, +) -> impl FnMut(&'i [u8]) -> ParseResult<'i, Filter<'i>, LdapError> { + move |bytes: &'i [u8]| Filter::from_ber_recursive(bytes, limit) +} + +impl<'a> Filter<'a> { + /// Parse a `Filter`, but with recursion limit. + /// + /// If `limit` reaches zero, returns an error `LdapError::FilterMaxDepth`. + fn from_ber_recursive(bytes: &'a [u8], limit: usize) -> ParseResult<'a, Self, LdapError> { + if limit == 0 { + return Err(Err::Error(LdapError::FilterMaxDepth)); + } + // new limit + let limit = limit - 1; + // read next element as ANY and look tag value let (rem, any) = Any::from_ber(bytes).map_err(Err::convert)?; // eprintln!("parse_ldap_filter: [{}] {:?}", header.tag.0, header); @@ -132,14 +159,14 @@ impl<'a> FromBer<'a, LdapError> for Filter<'a> { let content = any.data; let (_, filter) = match any.tag().0 { 0 => { - let (rem, sub_filters) = many1(complete(Filter::from_ber))(content)?; + let (rem, sub_filters) = many1(complete(filter_from_ber(limit)))(content)?; Ok((rem, Filter::And(sub_filters))) } 1 => { - let (rem, sub_filters) = many1(complete(Filter::from_ber))(content)?; + let (rem, sub_filters) = many1(complete(filter_from_ber(limit)))(content)?; Ok((rem, Filter::Or(sub_filters))) } - 2 => map(Filter::from_ber, |f| Filter::Not(Box::new(f)))(content), + 2 => map(filter_from_ber(limit), |f| Filter::Not(Box::new(f)))(content), 3 => map( parse_ldap_attribute_value_assertion_content, Filter::EqualityMatch, diff --git a/src/ldap.rs b/src/ldap.rs index e4332a7..554785d 100644 --- a/src/ldap.rs +++ b/src/ldap.rs @@ -6,6 +6,9 @@ use asn1_rs::{FromBer, ToStatic}; use rusticata_macros::newtype_enum; use std::borrow::Cow; +/// Hard limit for maximum recursion depth when parsing LDAP `Filter` +pub const MAX_FILTER_DEPTH: usize = 32; + #[derive(Debug, Default, PartialEq, Eq, PartialOrd, Ord, Clone, Copy, ToStatic)] pub struct ProtocolOpTag(pub u32); diff --git a/tests/ldap_filter.rs b/tests/ldap_filter.rs new file mode 100644 index 0000000..455d0e2 --- /dev/null +++ b/tests/ldap_filter.rs @@ -0,0 +1,27 @@ +use asn1_rs::FromBer; +use ldap_parser::{filter::Filter, ldap::MAX_FILTER_DEPTH}; + +fn encode_recursive_filter(depth: usize) -> Vec { + // manually encoded Filter::Present("test") + let f0 = b"\xA7\x04test"; + + // Wrap in "Not" until depth is reached + (1..depth).fold(f0.to_vec(), |acc, _| { + let len = acc.len(); + assert!(len < 127); + let mut v = vec![0xA2, len as u8]; + v.extend(acc); + v + }) +} + +#[test] +fn ldap_filter_recursion() { + // Allowed depth + let d = encode_recursive_filter(MAX_FILTER_DEPTH); + let _ = Filter::from_ber(&d).expect("Valid Filter depth"); + + // Invalid depth (above limit) + let d = encode_recursive_filter(MAX_FILTER_DEPTH + 1); + let _ = Filter::from_ber(&d).expect_err("Above max Filter depth"); +}