]> git.ipfire.org Git - thirdparty/suricata.git/commitdiff
detect/flow: move keyword parsing code to rust
authorPhilippe Antoine <pantoine@oisf.net>
Mon, 17 Feb 2025 11:49:59 +0000 (12:49 +0100)
committerVictor Julien <victor@inliniac.net>
Wed, 19 Feb 2025 08:21:33 +0000 (09:21 +0100)
for flow.pkts and flow.bytes keywords

Ticket: 7562

Avoid null deref when parsing flow.bytes:toserver;

rust/src/detect/flow.rs [new file with mode: 0644]
rust/src/detect/mod.rs
src/detect-flow-pkts.c

diff --git a/rust/src/detect/flow.rs b/rust/src/detect/flow.rs
new file mode 100644 (file)
index 0000000..96f68ed
--- /dev/null
@@ -0,0 +1,186 @@
+/* Copyright (C) 2025 Open Information Security Foundation
+ *
+ * You can copy, redistribute or modify this Program under the terms of
+ * the GNU General Public License version 2 as published by the Free
+ * Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * version 2 along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+ * 02110-1301, USA.
+ */
+
+use super::uint::{detect_parse_uint, DetectUintData};
+use nom7::branch::alt;
+use nom7::bytes::complete::{is_a, tag};
+use nom7::combinator::{opt, value};
+use nom7::IResult;
+use std::ffi::CStr;
+
+#[allow(non_camel_case_types)]
+#[derive(PartialEq, Eq, Clone, Debug)]
+#[repr(u8)]
+/// This data structure is also used in detect-flow-pkts.c
+pub enum DetectFlowDir {
+    DETECT_FLOW_TOSERVER = 1,
+    DETECT_FLOW_TOCLIENT = 2,
+    DETECT_FLOW_TOEITHER = 3,
+}
+
+#[derive(Debug, PartialEq)]
+#[repr(C)]
+/// This data structure is also used in detect-flow-pkts.c
+pub struct DetectFlowPkts {
+    pub pkt_data: DetectUintData<u32>,
+    pub dir: DetectFlowDir,
+}
+
+#[derive(Debug, PartialEq)]
+#[repr(C)]
+/// This data structure is also used in detect-flow-pkts.c
+pub struct DetectFlowBytes {
+    pub byte_data: DetectUintData<u64>,
+    pub dir: DetectFlowDir,
+}
+
+fn detect_parse_flow_direction(i: &str) -> IResult<&str, DetectFlowDir> {
+    let (i, fd) = alt((
+        value(DetectFlowDir::DETECT_FLOW_TOSERVER, tag("toserver")),
+        value(DetectFlowDir::DETECT_FLOW_TOCLIENT, tag("toclient")),
+        value(DetectFlowDir::DETECT_FLOW_TOEITHER, tag("either")),
+    ))(i)?;
+    return Ok((i, fd));
+}
+
+fn detect_parse_flow_pkts(i: &str) -> IResult<&str, DetectFlowPkts> {
+    let (i, _) = opt(is_a(" \t"))(i)?;
+    let (i, fd) = detect_parse_flow_direction(i)?;
+    let (i, _) = opt(is_a(" \t"))(i)?;
+    let (i, _) = tag(",")(i)?;
+    let (i, _) = opt(is_a(" \t"))(i)?;
+    return detect_parse_flow_pkts_dir(i, fd);
+}
+
+fn detect_parse_flow_pkts_dir(i: &str, dir: DetectFlowDir) -> IResult<&str, DetectFlowPkts> {
+    let (i, du32) = detect_parse_uint::<u32>(i)?;
+    return Ok((
+        i,
+        DetectFlowPkts {
+            pkt_data: du32,
+            dir,
+        },
+    ));
+}
+
+#[no_mangle]
+pub unsafe extern "C" fn SCDetectFlowPktsParseDir(
+    ustr: *const std::os::raw::c_char, dir: DetectFlowDir,
+) -> *mut DetectFlowPkts {
+    let ft_name: &CStr = CStr::from_ptr(ustr); //unsafe
+    if let Ok(s) = ft_name.to_str() {
+        if let Ok((_, ctx)) = detect_parse_flow_pkts_dir(s, dir) {
+            let boxed = Box::new(ctx);
+            return Box::into_raw(boxed) as *mut _;
+        }
+    }
+    return std::ptr::null_mut();
+}
+
+#[no_mangle]
+pub unsafe extern "C" fn SCDetectFlowPktsParse(
+    ustr: *const std::os::raw::c_char,
+) -> *mut DetectFlowPkts {
+    let ft_name: &CStr = CStr::from_ptr(ustr); //unsafe
+    if let Ok(s) = ft_name.to_str() {
+        if let Ok((_, ctx)) = detect_parse_flow_pkts(s) {
+            let boxed = Box::new(ctx);
+            return Box::into_raw(boxed) as *mut _;
+        }
+    }
+    return std::ptr::null_mut();
+}
+
+#[no_mangle]
+pub unsafe extern "C" fn SCDetectFlowPktsFree(ctx: &mut DetectFlowPkts) {
+    std::mem::drop(Box::from_raw(ctx));
+}
+
+fn detect_parse_flow_bytes(i: &str) -> IResult<&str, DetectFlowBytes> {
+    let (i, _) = opt(is_a(" \t"))(i)?;
+    let (i, fd) = detect_parse_flow_direction(i)?;
+    let (i, _) = opt(is_a(" \t"))(i)?;
+    let (i, _) = tag(",")(i)?;
+    let (i, _) = opt(is_a(" \t"))(i)?;
+    return detect_parse_flow_bytes_dir(i, fd);
+}
+
+fn detect_parse_flow_bytes_dir(i: &str, dir: DetectFlowDir) -> IResult<&str, DetectFlowBytes> {
+    let (i, du64) = detect_parse_uint::<u64>(i)?;
+    return Ok((
+        i,
+        DetectFlowBytes {
+            byte_data: du64,
+            dir,
+        },
+    ));
+}
+
+#[no_mangle]
+pub unsafe extern "C" fn SCDetectFlowBytesParseDir(
+    ustr: *const std::os::raw::c_char, dir: DetectFlowDir,
+) -> *mut DetectFlowBytes {
+    let ft_name: &CStr = CStr::from_ptr(ustr); //unsafe
+    if let Ok(s) = ft_name.to_str() {
+        if let Ok((_, ctx)) = detect_parse_flow_bytes_dir(s, dir) {
+            let boxed = Box::new(ctx);
+            return Box::into_raw(boxed) as *mut _;
+        }
+    }
+    return std::ptr::null_mut();
+}
+
+#[no_mangle]
+pub unsafe extern "C" fn SCDetectFlowBytesParse(
+    ustr: *const std::os::raw::c_char,
+) -> *mut DetectFlowBytes {
+    let ft_name: &CStr = CStr::from_ptr(ustr); //unsafe
+    if let Ok(s) = ft_name.to_str() {
+        if let Ok((_, ctx)) = detect_parse_flow_bytes(s) {
+            let boxed = Box::new(ctx);
+            return Box::into_raw(boxed) as *mut _;
+        }
+    }
+    return std::ptr::null_mut();
+}
+
+#[no_mangle]
+pub unsafe extern "C" fn SCDetectFlowBytesFree(ctx: &mut DetectFlowBytes) {
+    std::mem::drop(Box::from_raw(ctx));
+}
+
+#[cfg(test)]
+mod test {
+    use super::*;
+    use crate::detect::uint::DetectUintMode;
+
+    #[test]
+    fn test_detect_parse_flow_pkts() {
+        assert_eq!(
+            detect_parse_flow_pkts(" toserver  ,   300 ").unwrap().1,
+            DetectFlowPkts {
+                pkt_data: DetectUintData {
+                    arg1: 300,
+                    arg2: 0,
+                    mode: DetectUintMode::DetectUintModeEqual,
+                },
+                dir: DetectFlowDir::DETECT_FLOW_TOSERVER,
+            }
+        );
+        assert!(detect_parse_flow_pkts("toserver").is_err());
+    }
+}
index b040014c441d88ab712f63a5ad84c466b522eec2..9a5de6dad8488af33b6083b68b90eae5af5f23f3 100644 (file)
@@ -20,6 +20,7 @@
 pub mod byte_extract;
 pub mod byte_math;
 pub mod error;
+pub mod flow;
 pub mod iprep;
 pub mod parser;
 pub mod requires;
index 0ed1b487750dad855257059553405127ec783410..d223c1c5b30b1b0c6dff339b5e8c5413a1480e9d 100644 (file)
 #include "detect-engine-uint.h"
 #include "detect-parse.h"
 
-enum FlowDirection {
-    DETECT_FLOW_TOSERVER = 1,
-    DETECT_FLOW_TOCLIENT,
-    DETECT_FLOW_TOEITHER,
-};
-
-typedef struct DetectFlowPkts_ {
-    DetectU32Data *pkt_data;
-    enum FlowDirection dir;
-} DetectFlowPkts;
-
-typedef struct DetectFlowBytes_ {
-    DetectU64Data *byte_data;
-    enum FlowDirection dir;
-} DetectFlowBytes;
-
 static int DetectFlowPktsMatch(
         DetectEngineThreadCtx *det_ctx, Packet *p, const Signature *s, const SigMatchCtx *ctx)
 {
@@ -48,42 +32,32 @@ static int DetectFlowPktsMatch(
 
     const DetectFlowPkts *df = (const DetectFlowPkts *)ctx;
     if (df->dir == DETECT_FLOW_TOSERVER) {
-        return DetectU32Match(p->flow->todstpktcnt, df->pkt_data);
+        return DetectU32Match(p->flow->todstpktcnt, &df->pkt_data);
     } else if (df->dir == DETECT_FLOW_TOCLIENT) {
-        return DetectU32Match(p->flow->tosrcpktcnt, df->pkt_data);
+        return DetectU32Match(p->flow->tosrcpktcnt, &df->pkt_data);
     } else if (df->dir == DETECT_FLOW_TOEITHER) {
-        if (DetectU32Match(p->flow->tosrcpktcnt, df->pkt_data)) {
+        if (DetectU32Match(p->flow->tosrcpktcnt, &df->pkt_data)) {
             return 1;
         }
-        return DetectU32Match(p->flow->todstpktcnt, df->pkt_data);
+        return DetectU32Match(p->flow->todstpktcnt, &df->pkt_data);
     }
     return 0;
 }
 
 static void DetectFlowPktsFree(DetectEngineCtx *de_ctx, void *ptr)
 {
-    DetectFlowPkts *df = (DetectFlowPkts *)ptr;
-    if (df != NULL) {
-        rs_detect_u32_free(df->pkt_data);
-        SCFree(df);
+    if (ptr != NULL) {
+        SCDetectFlowPktsFree(ptr);
     }
 }
 
 static int DetectFlowPktsToServerSetup(DetectEngineCtx *de_ctx, Signature *s, const char *rawstr)
 {
-    DetectU32Data *du32 = DetectU32Parse(rawstr);
-    if (du32 == NULL)
-        return -1;
-
-    DetectFlowPkts *df = SCCalloc(1, sizeof(DetectFlowPkts));
+    DetectFlowPkts *df = SCDetectFlowPktsParseDir(rawstr, DETECT_FLOW_TOSERVER);
     if (df == NULL) {
-        rs_detect_u32_free(du32);
         return -1;
     }
 
-    df->pkt_data = du32;
-    df->dir = DETECT_FLOW_TOSERVER;
-
     if (SigMatchAppendSMToList(
                 de_ctx, s, DETECT_FLOW_PKTS, (SigMatchCtx *)df, DETECT_SM_LIST_MATCH) == NULL) {
         DetectFlowPktsFree(de_ctx, df);
@@ -96,18 +70,10 @@ static int DetectFlowPktsToServerSetup(DetectEngineCtx *de_ctx, Signature *s, co
 
 static int DetectFlowPktsToClientSetup(DetectEngineCtx *de_ctx, Signature *s, const char *rawstr)
 {
-    DetectU32Data *du32 = DetectU32Parse(rawstr);
-    if (du32 == NULL)
-        return -1;
-
-    DetectFlowPkts *df = SCCalloc(1, sizeof(DetectFlowPkts));
+    DetectFlowPkts *df = SCDetectFlowPktsParseDir(rawstr, DETECT_FLOW_TOCLIENT);
     if (df == NULL) {
-        rs_detect_u32_free(du32);
         return -1;
     }
-    df->pkt_data = du32;
-    df->dir = DETECT_FLOW_TOCLIENT;
-
     if (SigMatchAppendSMToList(
                 de_ctx, s, DETECT_FLOW_PKTS, (SigMatchCtx *)df, DETECT_SM_LIST_MATCH) == NULL) {
         DetectFlowPktsFree(de_ctx, df);
@@ -120,59 +86,10 @@ static int DetectFlowPktsToClientSetup(DetectEngineCtx *de_ctx, Signature *s, co
 
 static int DetectFlowPktsSetup(DetectEngineCtx *de_ctx, Signature *s, const char *rawstr)
 {
-    DetectFlowPkts *df = NULL;
-    char copy[strlen(rawstr) + 1];
-    strlcpy(copy, rawstr, sizeof(copy));
-    char *context = NULL;
-    char *token = strtok_r(copy, ",", &context);
-    uint8_t num_tokens = 0;
-    uint8_t dir = 0;
-    char *pkt_data = NULL;
-
-    while (token != NULL) {
-        if (num_tokens > 1)
-            return -1;
-
-        while (*token != '\0' && isblank(*token)) {
-            token++;
-        }
-        if (strlen(token) == 0) {
-            goto next;
-        }
-
-        num_tokens++;
-
-        if (dir == 0 && num_tokens == 1) {
-            if (strcmp(token, "toserver") == 0) {
-                dir = DETECT_FLOW_TOSERVER;
-            } else if (strcmp(token, "toclient") == 0) {
-                dir = DETECT_FLOW_TOCLIENT;
-            } else if (strcmp(token, "either") == 0) {
-                dir = DETECT_FLOW_TOEITHER;
-            } else {
-                SCLogError("Invalid direction given: %s", token);
-                return -1;
-            }
-        }
-
-        if (dir && num_tokens == 2) {
-            pkt_data = token;
-        }
-
-    next:
-        token = strtok_r(NULL, ",", &context);
-    }
-
-    DetectU32Data *du32 = DetectU32Parse(pkt_data);
-    if (du32 == NULL)
-        return -1;
-    df = SCCalloc(1, sizeof(DetectFlowPkts));
+    DetectFlowPkts *df = SCDetectFlowPktsParse(rawstr);
     if (df == NULL) {
-        rs_detect_u32_free(du32);
         return -1;
     }
-    df->dir = dir;
-    df->pkt_data = du32;
     if (SigMatchAppendSMToList(
                 de_ctx, s, DETECT_FLOW_PKTS, (SigMatchCtx *)df, DETECT_SM_LIST_MATCH) == NULL) {
         DetectFlowPktsFree(de_ctx, df);
@@ -187,7 +104,7 @@ static int DetectFlowPktsSetup(DetectEngineCtx *de_ctx, Signature *s, const char
 static void PrefilterPacketFlowPktsSet(PrefilterPacketHeaderValue *v, void *smctx)
 {
     const DetectFlowPkts *df = smctx;
-    const DetectUintData_u32 *data = df->pkt_data;
+    const DetectUintData_u32 *data = &df->pkt_data;
     v->u8[0] = data->mode;
     v->u8[1] = (uint8_t)df->dir;
     v->u32[1] = data->arg1;
@@ -197,8 +114,8 @@ static void PrefilterPacketFlowPktsSet(PrefilterPacketHeaderValue *v, void *smct
 static bool PrefilterPacketFlowPktsCompare(PrefilterPacketHeaderValue v, void *smctx)
 {
     const DetectFlowPkts *df = smctx;
-    if (v.u8[0] == df->pkt_data->mode && v.u8[1] == df->dir && v.u32[1] == df->pkt_data->arg1 &&
-            v.u32[2] == df->pkt_data->arg2) {
+    if (v.u8[0] == df->pkt_data.mode && v.u8[1] == df->dir && v.u32[1] == df->pkt_data.arg1 &&
+            v.u32[2] == df->pkt_data.arg2) {
         return true;
     }
     return false;
@@ -215,7 +132,7 @@ static void PrefilterPacketFlowPktsMatch(
     DetectUintData_u32 data = {
         .mode = ctx->v1.u8[0], .arg1 = ctx->v1.u32[1], .arg2 = ctx->v1.u32[2]
     };
-    df.pkt_data = &data;
+    df.pkt_data = data;
     df.dir = ctx->v1.u8[1];
 
     if (DetectFlowPktsMatch(det_ctx, p, NULL, (const SigMatchCtx *)&df)) {
@@ -282,40 +199,31 @@ static int DetectFlowBytesMatch(
 
     const DetectFlowBytes *df = (const DetectFlowBytes *)ctx;
     if (df->dir == DETECT_FLOW_TOSERVER) {
-        return DetectU64Match(p->flow->todstbytecnt, df->byte_data);
+        return DetectU64Match(p->flow->todstbytecnt, &df->byte_data);
     } else if (df->dir == DETECT_FLOW_TOCLIENT) {
-        return DetectU64Match(p->flow->tosrcbytecnt, df->byte_data);
+        return DetectU64Match(p->flow->tosrcbytecnt, &df->byte_data);
     } else if (df->dir == DETECT_FLOW_TOEITHER) {
-        if (DetectU64Match(p->flow->tosrcbytecnt, df->byte_data)) {
+        if (DetectU64Match(p->flow->tosrcbytecnt, &df->byte_data)) {
             return 1;
         }
-        return DetectU64Match(p->flow->todstbytecnt, df->byte_data);
+        return DetectU64Match(p->flow->todstbytecnt, &df->byte_data);
     }
     return 0;
 }
 
 static void DetectFlowBytesFree(DetectEngineCtx *de_ctx, void *ptr)
 {
-    DetectFlowBytes *df = (DetectFlowBytes *)ptr;
-    if (df != NULL) {
-        rs_detect_u64_free(df->byte_data);
-        SCFree(df);
+    if (ptr != NULL) {
+        SCDetectFlowBytesFree(ptr);
     }
 }
 
 static int DetectFlowBytesToServerSetup(DetectEngineCtx *de_ctx, Signature *s, const char *rawstr)
 {
-    DetectU64Data *du64 = DetectU64Parse(rawstr);
-    if (du64 == NULL)
-        return -1;
-
-    DetectFlowBytes *df = SCCalloc(1, sizeof(DetectFlowBytes));
+    DetectFlowBytes *df = SCDetectFlowBytesParseDir(rawstr, DETECT_FLOW_TOSERVER);
     if (df == NULL) {
-        rs_detect_u64_free(du64);
         return -1;
     }
-    df->byte_data = du64;
-    df->dir = DETECT_FLOW_TOSERVER;
 
     if (SigMatchAppendSMToList(
                 de_ctx, s, DETECT_FLOW_BYTES, (SigMatchCtx *)df, DETECT_SM_LIST_MATCH) == NULL) {
@@ -329,19 +237,11 @@ static int DetectFlowBytesToServerSetup(DetectEngineCtx *de_ctx, Signature *s, c
 
 static int DetectFlowBytesToClientSetup(DetectEngineCtx *de_ctx, Signature *s, const char *rawstr)
 {
-    DetectU64Data *du64 = DetectU64Parse(rawstr);
-    if (du64 == NULL)
-        return -1;
-
-    DetectFlowBytes *df = SCCalloc(1, sizeof(DetectFlowBytes));
+    DetectFlowBytes *df = SCDetectFlowBytesParseDir(rawstr, DETECT_FLOW_TOCLIENT);
     if (df == NULL) {
-        rs_detect_u64_free(du64);
         return -1;
     }
 
-    df->byte_data = du64;
-    df->dir = DETECT_FLOW_TOCLIENT;
-
     if (SigMatchAppendSMToList(
                 de_ctx, s, DETECT_FLOW_BYTES, (SigMatchCtx *)df, DETECT_SM_LIST_MATCH) == NULL) {
         DetectFlowBytesFree(de_ctx, df);
@@ -354,59 +254,10 @@ static int DetectFlowBytesToClientSetup(DetectEngineCtx *de_ctx, Signature *s, c
 
 static int DetectFlowBytesSetup(DetectEngineCtx *de_ctx, Signature *s, const char *rawstr)
 {
-    DetectFlowBytes *df = NULL;
-    char copy[strlen(rawstr) + 1];
-    strlcpy(copy, rawstr, sizeof(copy));
-    char *context = NULL;
-    char *token = strtok_r(copy, ",", &context);
-    uint8_t num_tokens = 0;
-    uint8_t dir = 0;
-    char *byte_data = NULL;
-
-    while (token != NULL) {
-        if (num_tokens > 1)
-            return -1;
-
-        while (*token != '\0' && isblank(*token)) {
-            token++;
-        }
-        if (strlen(token) == 0) {
-            goto next;
-        }
-
-        num_tokens++;
-
-        if (dir == 0 && num_tokens == 1) {
-            if (strcmp(token, "toserver") == 0) {
-                dir = DETECT_FLOW_TOSERVER;
-            } else if (strcmp(token, "toclient") == 0) {
-                dir = DETECT_FLOW_TOCLIENT;
-            } else if (strcmp(token, "either") == 0) {
-                dir = DETECT_FLOW_TOEITHER;
-            } else {
-                SCLogError("Invalid direction given: %s", token);
-                return -1;
-            }
-        }
-
-        if (dir && num_tokens == 2) {
-            byte_data = token;
-        }
-
-    next:
-        token = strtok_r(NULL, ",", &context);
-    }
-
-    DetectU64Data *du64 = DetectU64Parse(byte_data);
-    if (du64 == NULL)
-        return -1;
-    df = SCCalloc(1, sizeof(DetectFlowBytes));
+    DetectFlowBytes *df = SCDetectFlowBytesParse(rawstr);
     if (df == NULL) {
-        rs_detect_u64_free(du64);
         return -1;
     }
-    df->dir = dir;
-    df->byte_data = du64;
     if (SigMatchAppendSMToList(
                 de_ctx, s, DETECT_FLOW_BYTES, (SigMatchCtx *)df, DETECT_SM_LIST_MATCH) == NULL) {
         DetectFlowBytesFree(de_ctx, df);