Fix/filter max depth

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

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 <chifflier@wzdftpd.net>"]
 edition = "2018"

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!("<empty>");
     }
-    print!("{}", &bytes[..m].to_hex(16));
+    print!("{}", bytes[..m].to_hex(16));
     if bytes.len() > max_len {
         println!("... <continued>");
     }

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,

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);
 

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<u8> {
+    // 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");
+}