From a4f21715608b75fe08a3cb02d4091ca57c2e06d9 Mon Sep 17 00:00:00 2001 From: Philippe Antoine Date: Mon, 17 Feb 2025 12:49:59 +0100 Subject: [PATCH] detect/flow: move keyword parsing code to rust for flow.pkts and flow.bytes keywords Ticket: 7562 Avoid null deref when parsing flow.bytes:toserver; --- rust/src/detect/flow.rs | 186 ++++++++++++++++++++++++++++++++++++++ rust/src/detect/mod.rs | 1 + src/detect-flow-pkts.c | 193 +++++----------------------------------- 3 files changed, 209 insertions(+), 171 deletions(-) create mode 100644 rust/src/detect/flow.rs diff --git a/rust/src/detect/flow.rs b/rust/src/detect/flow.rs new file mode 100644 index 000000000000..96f68edba7de --- /dev/null +++ b/rust/src/detect/flow.rs @@ -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, + 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, + 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::(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::(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()); + } +} diff --git a/rust/src/detect/mod.rs b/rust/src/detect/mod.rs index b040014c441d..9a5de6dad848 100644 --- a/rust/src/detect/mod.rs +++ b/rust/src/detect/mod.rs @@ -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; diff --git a/src/detect-flow-pkts.c b/src/detect-flow-pkts.c index 0ed1b487750d..d223c1c5b30b 100644 --- a/src/detect-flow-pkts.c +++ b/src/detect-flow-pkts.c @@ -23,22 +23,6 @@ #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);