detect/asn1: Update relative_offset keyword

- To be consistent with recent C version changes
- Add checks for over/underflows

diff --git a/rust/src/asn1/mod.rs b/rust/src/asn1/mod.rs
index cadbc0c1644be469f0b0f134d89ee373205b6866..9486ce843daf812baddcf34c21d1bd58c3ab6778 100644 (file)
--- a/rust/src/asn1/mod.rs
+++ b/rust/src/asn1/mod.rs
@@ -192,36 +192,40 @@ impl Asn1 {
 /// Decodes Asn1 objects from an input + length while applying the offset
 /// defined in the asn1 keyword options
 fn asn1_decode(
-    input: *const u8,
-    input_len: u32,
+    buffer: &'static [u8],
+    buffer_offset: u32,
     ad: &DetectAsn1Data,
 ) -> Result<Asn1, Asn1DecodeError> {
     // Get offset
     let offset = if let Some(absolute_offset) = ad.absolute_offset {
-        absolute_offset as isize
+        absolute_offset
     } else if let Some(relative_offset) = ad.relative_offset {
-        relative_offset as isize
+        // relative offset in regards to the last content match
+
+        // buffer_offset (u32) + relative_offset (i32) => offset (u16)
+        u16::try_from({
+            if relative_offset > 0 {
+                buffer_offset
+                    .checked_add(u32::try_from(relative_offset)?)
+                    .ok_or(Asn1DecodeError::InvalidKeywordParameter)?
+            } else {
+                buffer_offset
+                    .checked_sub(u32::try_from(-relative_offset)?)
+                    .ok_or(Asn1DecodeError::InvalidKeywordParameter)?
+            }
+        })
+        .or(Err(Asn1DecodeError::InvalidKeywordParameter))?
     } else {
         0
     };
 
-    // Make sure we won't read past the end of the buffer
-    if offset >= input_len as isize {
+    // Make sure we won't read past the end or front of the buffer
+    if offset as usize >= buffer.len() {
         return Err(Asn1DecodeError::InvalidKeywordParameter);
     }
 
-    // Apply offset to input pointer
-    let input = unsafe { input.offset(offset) };
-
-    // Adjust the length
-    let input_len = (input_len as isize)
-        .checked_sub(offset)
-        .ok_or(Asn1DecodeError::InvalidKeywordParameter)?;
-    let input_len =
-        usize::try_from(input_len).map_err(|_| Asn1DecodeError::InvalidKeywordParameter)?;
-
-    // Get the slice from memory
-    let slice = build_slice!(input, input_len);
+    // Get slice from buffer at offset
+    let slice = &buffer[offset as usize..];
 
     Asn1::from_slice(slice, ad)
 }
@@ -236,16 +240,19 @@ fn asn1_decode(
 #[no_mangle]
 pub unsafe extern "C" fn rs_asn1_decode(
     input: *const u8,
-    input_len: u32,
+    input_len: u16,
+    buffer_offset: u32,
     ad_ptr: *const DetectAsn1Data,
 ) -> *mut Asn1 {
     if input.is_null() || input_len == 0 || ad_ptr.is_null() {
         return std::ptr::null_mut();
     }
 
+    let slice = build_slice!(input, input_len as usize);
+
     let ad = &*ad_ptr;
 
-    let res = asn1_decode(input, input_len, ad);
+    let res = asn1_decode(slice, buffer_offset, ad);
 
     match res {
         Ok(asn1) => Box::into_raw(Box::new(asn1)),
@@ -296,6 +303,12 @@ pub unsafe extern "C" fn rs_asn1_checks(
     0
 }
 
+impl From<std::num::TryFromIntError> for Asn1DecodeError {
+    fn from(_e: std::num::TryFromIntError) -> Asn1DecodeError {
+        Asn1DecodeError::InvalidKeywordParameter
+    }
+}
+
 impl From<nom::Err<der_parser::error::BerError>> for Asn1DecodeError {
     fn from(e: nom::Err<der_parser::error::BerError>) -> Asn1DecodeError {
         match e {

diff --git a/rust/src/asn1/parse_rules.rs b/rust/src/asn1/parse_rules.rs
index 592b8b6cac40c023d93edecef9887367e6d78dfb..a9c3b3f22d44a3b61969084481d8162604b6b83d 100644 (file)
--- a/rust/src/asn1/parse_rules.rs
+++ b/rust/src/asn1/parse_rules.rs
@@ -19,7 +19,7 @@ use crate::log::*;
 use nom::branch::alt;
 use nom::bytes::complete::tag;
 use nom::character::complete::{digit1, multispace0, multispace1};
-use nom::combinator::{map_res, opt};
+use nom::combinator::{map_res, opt, verify};
 use nom::sequence::{separated_pair, tuple};
 use nom::IResult;
 use std::ffi::CStr;
@@ -84,7 +84,7 @@ pub struct DetectAsn1Data {
     pub bitstring_overflow: bool,
     pub double_overflow: bool,
     pub oversize_length: Option<u32>,
-    pub absolute_offset: Option<u32>,
+    pub absolute_offset: Option<u16>,
     pub relative_offset: Option<i32>,
     pub max_frames: u16,
 }
@@ -105,6 +105,11 @@ impl Default for DetectAsn1Data {
 fn parse_u32_number(input: &str) -> IResult<&str, u32> {
     map_res(digit1, |digits: &str| digits.parse::<u32>())(input)
 }
+
+fn parse_u16_number(input: &str) -> IResult<&str, u16> {
+    map_res(digit1, |digits: &str| digits.parse::<u16>())(input)
+}
+
 fn parse_i32_number(input: &str) -> IResult<&str, i32> {
     let (rest, negate) = opt(tag("-"))(input)?;
     let (rest, d) = map_res(digit1, |s: &str| s.parse::<i32>())(rest)?;
@@ -135,12 +140,18 @@ pub(super) fn asn1_parse_rule(input: &str) -> IResult<&str, DetectAsn1Data> {
         separated_pair(tag("oversize_length"), multispace1, parse_u32_number)(i)
     }
 
-    fn absolute_offset(i: &str) -> IResult<&str, (&str, u32)> {
-        separated_pair(tag("absolute_offset"), multispace1, parse_u32_number)(i)
+    fn absolute_offset(i: &str) -> IResult<&str, (&str, u16)> {
+        separated_pair(tag("absolute_offset"), multispace1, parse_u16_number)(i)
     }
 
     fn relative_offset(i: &str) -> IResult<&str, (&str, i32)> {
-        separated_pair(tag("relative_offset"), multispace1, parse_i32_number)(i)
+        separated_pair(
+            tag("relative_offset"),
+            multispace1,
+            verify(parse_i32_number, |v| {
+                *v >= -i32::from(std::u16::MAX) && *v <= i32::from(std::u16::MAX)
+            }),
+        )(i)
     }
 
     let mut data = DetectAsn1Data::default();
@@ -202,6 +213,18 @@ mod tests {
     #[test_case("oversize_length 1024",
         DetectAsn1Data { oversize_length: Some(1024), ..Default::default()};
         "check that we parse oversize_length correctly")]
+    #[test_case("oversize_length 0",
+        DetectAsn1Data { oversize_length: Some(0), ..Default::default()};
+        "check lower bound on oversize_length")]
+    #[test_case("oversize_length -1",
+        DetectAsn1Data::default() => panics "Error((\"oversize_length -1\", Verify))";
+        "check under lower bound on oversize_length")]
+    #[test_case("oversize_length 4294967295",
+        DetectAsn1Data { oversize_length: Some(4294967295), ..Default::default()};
+        "check upper bound on oversize_length")]
+    #[test_case("oversize_length 4294967296",
+        DetectAsn1Data::default() => panics "Error((\"oversize_length 4294967296\", Verify))";
+        "check over upper bound on oversize_length")]
     #[test_case("oversize_length",
         DetectAsn1Data::default() => panics "Error((\"oversize_length\", Verify))";
         "check that we fail if the needed arg oversize_length is not given")]
@@ -209,6 +232,18 @@ mod tests {
     #[test_case("absolute_offset 1024",
         DetectAsn1Data { absolute_offset: Some(1024), ..Default::default()};
         "check that we parse absolute_offset correctly")]
+    #[test_case("absolute_offset 0",
+        DetectAsn1Data { absolute_offset: Some(0), ..Default::default()};
+        "check lower bound on absolute_offset")]
+    #[test_case("absolute_offset -1",
+        DetectAsn1Data::default() => panics "Error((\"absolute_offset -1\", Verify))";
+        "check under lower bound on absolute_offset")]
+    #[test_case("absolute_offset 65535",
+        DetectAsn1Data { absolute_offset: Some(65535), ..Default::default()};
+        "check upper bound on absolute_offset")]
+    #[test_case("absolute_offset 65536",
+        DetectAsn1Data::default() => panics "Error((\"absolute_offset 65536\", Verify))";
+        "check over upper bound on absolute_offset")]
     #[test_case("absolute_offset",
         DetectAsn1Data::default() => panics "Error((\"absolute_offset\", Verify))";
         "check that we fail if the needed arg absolute_offset is not given")]
@@ -216,6 +251,18 @@ mod tests {
     #[test_case("relative_offset 1024",
         DetectAsn1Data { relative_offset: Some(1024), ..Default::default()};
         "check that we parse relative_offset correctly")]
+    #[test_case("relative_offset -65535",
+        DetectAsn1Data { relative_offset: Some(-65535), ..Default::default()};
+        "check lower bound on relative_offset")]
+    #[test_case("relative_offset -65536",
+        DetectAsn1Data::default() => panics "Error((\"relative_offset -65536\", Verify))";
+        "check under lower bound on relative_offset")]
+    #[test_case("relative_offset 65535",
+        DetectAsn1Data { relative_offset: Some(65535), ..Default::default()};
+        "check upper bound on relative_offset")]
+    #[test_case("relative_offset 65536",
+        DetectAsn1Data::default() => panics "Error((\"relative_offset 65536\", Verify))";
+        "check over upper bound on relative_offset")]
     #[test_case("relative_offset",
         DetectAsn1Data::default() => panics "Error((\"relative_offset\", Verify))";
         "check that we fail if the needed arg relative_offset is not given")]

diff --git a/src/detect-asn1.c b/src/detect-asn1.c
index 964987da5c44954daa3580ddaa63f35fee323852..37c5e290ff6310a8a58c6dbee4bdf11494f85707 100644 (file)
--- a/src/detect-asn1.c
+++ b/src/detect-asn1.c
@@ -81,7 +81,7 @@ static int DetectAsn1Match(DetectEngineThreadCtx *det_ctx, Packet *p,
 
     const DetectAsn1Data *ad = (const DetectAsn1Data *)ctx;
 
-    Asn1 *asn1 = rs_asn1_decode(p->payload, p->payload_len, ad);
+    Asn1 *asn1 = rs_asn1_decode(p->payload, p->payload_len, det_ctx->buffer_offset, ad);
 
     ret = rs_asn1_checks(asn1, ad);
 
@@ -223,7 +223,7 @@ static int DetectAsn1TestReal01(void)
              "content:\"Pablo\"; asn1:absolute_offset 0, "
              "oversize_length 130; sid:1;)";
     sigs[1]= "alert ip any any -> any any (msg:\"Testing id 2\"; "
-             "content:\"AA\"; asn1:relative_offset 2, "
+             "content:\"AA\"; asn1:relative_offset 0, "
              "oversize_length 130; sid:2;)";
     sigs[2]= "alert ip any any -> any any (msg:\"Testing id 3\"; "
              "content:\"lalala\"; asn1: oversize_length 2000; sid:3;)";
@@ -302,7 +302,7 @@ static int DetectAsn1TestReal02(void)
              "content:\"Pablo\"; asn1:absolute_offset 0, "
              "oversize_length 140; sid:1;)";
     sigs[1]= "alert ip any any -> any any (msg:\"Testing id 2\"; "
-             "content:\"AA\"; asn1:relative_offset 2, "
+             "content:\"AA\"; asn1:relative_offset 0, "
              "oversize_length 140; sid:2;)";
     sigs[2]= "alert ip any any -> any any (msg:\"Testing id 3\"; "
              "content:\"lalala\"; asn1: oversize_length 2000; sid:3;)";
@@ -440,7 +440,7 @@ static int DetectAsn1TestReal04(void)
              "content:\"Pablo\"; asn1:absolute_offset 0, "
              "oversize_length 140; sid:1;)";
     sigs[1]= "alert ip any any -> any any (msg:\"Testing id 2\"; "
-             "content:\"John\"; asn1:relative_offset -7, "
+             "content:\"John\"; asn1:relative_offset -11, "
              "oversize_length 140; sid:2;)";
     sigs[2]= "alert ip any any -> any any (msg:\"Testing id 3\"; "
              "content:\"lalala\"; asn1: oversize_length 2000; sid:3;)";