From f496053253e8fc1520d816638704419a11bb7c27 Mon Sep 17 00:00:00 2001 From: Shivani Bhardwaj Date: Tue, 19 Aug 2025 16:39:15 +0530 Subject: [PATCH 1/4] rust: add new dependency petgraph Ticket: 7638 Ticket: 7771 Ticket: 1399 --- rust/Cargo.lock.in | 40 +++++++++++++++++++++++++++++++++++++--- rust/Cargo.toml.in | 2 ++ 2 files changed, 39 insertions(+), 3 deletions(-) diff --git a/rust/Cargo.lock.in b/rust/Cargo.lock.in index 7f2c93d997..41a91449cb 100644 --- a/rust/Cargo.lock.in +++ b/rust/Cargo.lock.in @@ -491,6 +491,12 @@ version = "0.1.9" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5baebc0774151f905a1a2cc41989300b1e6fbb29aff0ceffa1064fdd3088d582" +[[package]] +name = "fixedbitset" +version = "0.5.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1d674e81391d1e1ab681a28d99df07927c6d4aa5b027d7da16ba32d1d21ecd99" + [[package]] name = "flate2" version = "1.0.35" @@ -502,6 +508,12 @@ dependencies = [ "miniz_oxide", ] +[[package]] +name = "foldhash" +version = "0.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d9c4f5dac5e15c24eb999c26181a6ca40b39fe946cbe4c263c7209467bc83af2" + [[package]] name = "foldhash" version = "0.2.0" @@ -639,6 +651,15 @@ dependencies = [ "polyval", ] +[[package]] +name = "hashbrown" +version = "0.15.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9229cfe53dfd69f0609a49f65461bd93001ea1ef889cd5529dd176593f5338a1" +dependencies = [ + "foldhash 0.1.5", +] + [[package]] name = "hashbrown" version = "0.16.1" @@ -647,7 +668,7 @@ checksum = "841d1cc9bed7f9236f321df977030373f4a4163ae1a7dbfe1a51a2c1a51d9100" dependencies = [ "allocator-api2", "equivalent", - "foldhash", + "foldhash 0.2.0", ] [[package]] @@ -702,7 +723,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4b0f83760fb341a774ed326568e19f5a863af4a952def8c39f9ab92fd95b88e5" dependencies = [ "equivalent", - "hashbrown", + "hashbrown 0.16.1", ] [[package]] @@ -803,7 +824,7 @@ version = "0.16.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7f66e8d5d03f609abc3a39e6f08e4164ebf1447a732906d39eb9b99b7919ef39" dependencies = [ - "hashbrown", + "hashbrown 0.16.1", ] [[package]] @@ -1082,6 +1103,18 @@ version = "0.3.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c08d65885ee38876c4f86fa503fb49d7b507c2b62552df7c70b2fce627e06381" +[[package]] +name = "petgraph" +version = "0.8.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8701b58ea97060d5e5b155d383a69952a60943f0e6dfe30b04c287beb0b27455" +dependencies = [ + "fixedbitset", + "hashbrown 0.15.5", + "indexmap", + "serde", +] + [[package]] name = "phf" version = "0.10.1" @@ -1571,6 +1604,7 @@ dependencies = [ "num", "num-derive", "num-traits 0.2.19", + "petgraph", "psl", "regex", "sawp", diff --git a/rust/Cargo.toml.in b/rust/Cargo.toml.in index 6bab634a8f..9203e7a934 100644 --- a/rust/Cargo.toml.in +++ b/rust/Cargo.toml.in @@ -61,6 +61,8 @@ lru = "~0.16.3" der-parser = { version = "~9.0.0", default-features = false } kerberos-parser = { version = "~0.8.0", default-features = false } +petgraph = "~0.8.2" + sawp-modbus = "~0.13.1" sawp-pop3 = "~0.13.1" sawp = "~0.13.1" From 1e774460e55f0801367d1146a9a25e6a58c36e1c Mon Sep 17 00:00:00 2001 From: Shivani Bhardwaj Date: Mon, 20 Apr 2026 20:26:14 +0530 Subject: [PATCH 2/4] rust: add flowbit dependency checker This solution makes use of the petgraph crate to create a directed stable graph of signatures. The algorithm is as follows: - A stable Directed Graph is created - A Flowbit Analyzer array is created which stores the sids per flowbit per command - for each entry in the flowbit analyzer "READ" array of type isset - set array is walked => a directed edge* from the set sid to isset sid with the weight of command is added. - for each entry in the flowbit analyzer "READ" array of type isnotset - unset array is walked => a directed edge from the set sid to isnotset sid with the weight of command is added. - If there was no error (cycle), this means that the current graph is a DAG (Directed Acyclic Graph) - Report if any unresolvable cyclic dependencies are found. 1. If a cycle is formed by varying edge weights, it's an invalid cycle. This would be formed by usage of different "WRITE" commands in different signatures e.g. sid: 1 => set, A; isset, B; # a directed edge of weight 0 is created from 1 to 2 for flowbit A. sid: 2 => isset, A; unset, B; # a directed edge of weight 2 is created from 2 to 1 for flowbit B. 2. If the cycle is formed by the same edge weights and on the same flowbit, it is considered unsatisfiable at runtime. Bug 7771 --- rust/src/utils/flowbits_resolver.rs | 257 ++++++++++++++++++++++++++++ rust/src/utils/mod.rs | 3 +- 2 files changed, 259 insertions(+), 1 deletion(-) create mode 100644 rust/src/utils/flowbits_resolver.rs diff --git a/rust/src/utils/flowbits_resolver.rs b/rust/src/utils/flowbits_resolver.rs new file mode 100644 index 0000000000..885029799e --- /dev/null +++ b/rust/src/utils/flowbits_resolver.rs @@ -0,0 +1,257 @@ +/* Copyright (C) 2026 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. + */ + +// Author: Shivani Bhardwaj + +use petgraph::algo::{is_cyclic_directed, tarjan_scc}; +use petgraph::graph::{GraphError, NodeIndex}; +use petgraph::stable_graph::StableDiGraph; +use petgraph::Direction; +use std::collections::{HashMap, HashSet}; +use std::os::raw::c_void; + +/// Special Graph Node storing flowbit or signature +#[derive(Debug, Copy, Clone)] +struct SCGNode { + sid: u32, + nidx: NodeIndex, /* Graph's internal node index */ +} + +/// Edge object to determine a genuine cycle breaker +#[derive(Debug, Copy, Clone)] +struct SCGEdge { + cmd: u8, + fb_id: u32, +} + +#[derive(Debug)] +struct FlowbitSidStore { + graph: StableDiGraph, + iid_map: HashMap, + edge_set: HashSet<(u32, u32, u8, u32)>, +} + +/// Function to create an empty directed Graph +#[no_mangle] +pub unsafe extern "C" fn SCCreateDirectedGraph() -> *mut c_void { + // StableDiGraph is the ideal choice here for there is removal of + // nodes in the line later and this type of graph guarantees to + // not re-use any existing node indices + let fb_sid_store = FlowbitSidStore { + graph: StableDiGraph::new(), + iid_map: HashMap::new(), + edge_set: HashSet::new(), + }; + + /* Make an opaque pointer for C as nothing is changed there */ + return Box::into_raw(Box::new(fb_sid_store)) as *mut c_void; +} + +/// Drop the directed Graph. Called from C. +#[no_mangle] +pub unsafe extern "C" fn SCFreeDirectedGraph(store: *mut c_void) { + let _ = Box::from_raw(store as *mut FlowbitSidStore); +} + +#[no_mangle] +pub unsafe extern "C" fn SCGetOrCreateNodeGraph(graph: *mut c_void, iid: u32, sid: u32) -> i64 { + let g = &mut *(graph as *mut FlowbitSidStore); + + let Some(node_idx) = get_or_create_node(g, iid, sid) else { + SCLogError!("Error adding node; Graph is at full capacity"); + return -2; + }; + + node_idx.index() as i64 +} + +/// Function to get or create a node and add an appropriate directed +/// edge based on its type +#[no_mangle] +pub unsafe extern "C" fn SCCreateNodeEdgeDirectedGraph( + fss_void: *mut c_void, from: u32, to: u32, cmd: u8, fb_id: u32, +) -> i64 { + let fss = &mut *(fss_void as *mut FlowbitSidStore); + + let from_idx = NodeIndex::from(from); + let to_idx = NodeIndex::from(to); + + if !fss.edge_set.insert((from, to, cmd, fb_id)) { + SCLogDebug!( + "Skipped duplicate edge from {:?} -> {:?} for flowbit {:?} with command: {:?}", + from, + to, + fb_id, + cmd + ); + return 0; + } + + match fss + .graph + .try_add_edge(from_idx, to_idx, SCGEdge { cmd, fb_id }) + { + /* edge from a flowbit setter to a flowbit reader */ + Ok(_) => { + SCLogDebug!( + "Created an edge from {:?} -> {:?} for flowbit {:?} with command: {:?}", + from, + to, + fb_id, + cmd + ); + } + Err(GraphError::EdgeIxLimit) => { + SCLogError!("Error adding edge; Graph is at full capacity"); + return -2; + } + Err(GraphError::NodeOutBounds) => { + SCLogError!("Error adding edge; node does not exist"); + return -2; + } + Err(_) => { + SCLogError!("Error adding edge to the Graph"); + return -2; + } + } + + return 0; +} + +fn check_cycle_update_graph(graph: &mut StableDiGraph) -> i8 { + let max_stack_size: usize = 100; + + /* A self loop comes from a signature checking a flowbit it sets itself. + * It never blocks that signature, so remove all of them upfront instead + * of spending a try on each one */ + graph.retain_edges(|g, e| g.edge_endpoints(e).is_none_or(|(src, tgt)| src != tgt)); + + for _i in 0..max_stack_size { + /* Check graph for any cycles */ + if !is_cyclic_directed(&*graph) { + SCLogDebug!("no cycles after {} tries", _i); + return 0; + } + + SCLogDebug!("Found a cycle in i {}. Checking if it's valid..", _i); + + if !try_resolve_one_cycle(graph) { + /* If we can't resolve any cycle, we're stuck */ + SCLogDebug!("Unable to resolve cycles after {} tries", _i); + return -1; + } + } + + SCLogDebug!( + "Maximum tries ({}) reached while trying to resolve cycles", + max_stack_size + ); + return -1; +} + +fn try_resolve_one_cycle(graph: &mut StableDiGraph) -> bool { + let sccs = tarjan_scc(&*graph); + let mut edge_map: HashMap = HashMap::new(); + + /* Find the first multi-node SCC; self loops were already removed so a + * single node SCC is never part of a cycle */ + for scc in sccs { + if scc.len() == 1 { + continue; + } + + SCLogDebug!("Current scc: {:?}", scc); + let scc_set: HashSet<_> = scc.iter().copied().collect(); + let edge_indices: Vec<_> = graph.edge_indices().collect(); + + for edge_idx in edge_indices { + let Some((_src, tgt)) = graph.edge_endpoints(edge_idx) else { + continue; + }; + if !(scc_set.contains(&_src) && scc_set.contains(&tgt)) { + continue; + } + let SCGEdge { cmd, fb_id } = graph[edge_idx]; + let same_bit_setters = graph + .edges_directed(tgt, Direction::Incoming) + .filter(|e| e.weight().fb_id == fb_id) + .count(); + if same_bit_setters > 1 { + graph.remove_edge(edge_idx); + return true; + } + /* store cycle edges to inspect their commands (weights) later */ + edge_map.insert(edge_idx, cmd); + } + /* Find if the cycle causing edges are made up of differing weights */ + let distinct_weights = edge_map.values().copied().collect::>().len(); + debug_validate_bug_on!(distinct_weights == 0); + if distinct_weights > 1 { + /* Find and remove the edge with highest weight (lowest priority) */ + if let Some((cur_e, _)) = edge_map.iter().max_by_key(|(_, &weight)| weight) { + graph.remove_edge(*cur_e); + return true; + } + } else { + /* Valid cycle with same weights -- can't resolve */ + let mut sids: Vec<_> = scc_set.iter().map(|a| graph[*a].sid).collect(); + sids.sort_unstable(); + SCLogError!( + "Cyclic dependency found between flowbits from signatures: {:?}", + sids + ); + return false; + } + break; + } + + /* couldn't resolve */ + false +} + +/// Check the graph for flowbit dependencies that cannot be met at runtime +#[no_mangle] +pub unsafe extern "C" fn SCCheckFlowbitDependencies(fss_void: *mut c_void) -> i8 { + SCLogDebug!("Checking flowbit dependencies"); + let fss = &mut *(fss_void as *mut FlowbitSidStore); + + if check_cycle_update_graph(&mut fss.graph) == -1 { + SCLogDebug!("Couldn't do anything to fix the graph. Retreating.."); + return -1; + } + + return 0; +} + +fn get_or_create_node(fss: &mut FlowbitSidStore, iid: u32, sid: u32) -> Option { + if let Some(&nidx) = fss.iid_map.get(&iid) { + return Some(nidx); /* O(1) */ + } + let nd = SCGNode { + sid, + nidx: NodeIndex::from(u32::MAX), + }; + if let Ok(idx) = fss.graph.try_add_node(nd) { + /* O(1) */ + fss.graph[idx].nidx = idx; + fss.iid_map.insert(iid, idx); + SCLogDebug!("Created node: {:?}", fss.graph[idx]); + return Some(idx); + } + + None +} diff --git a/rust/src/utils/mod.rs b/rust/src/utils/mod.rs index 542aa7791f..36aea88ec4 100644 --- a/rust/src/utils/mod.rs +++ b/rust/src/utils/mod.rs @@ -1,4 +1,4 @@ -/* Copyright (C) 2024 Open Information Security Foundation +/* Copyright (C) 2024-2026 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 @@ -17,4 +17,5 @@ pub mod base64; pub mod datalink; +pub mod flowbits_resolver; pub mod ipaddr; From 41d2ad0acf59a79bf849bcf777fb9de581234e08 Mon Sep 17 00:00:00 2001 From: Shivani Bhardwaj Date: Wed, 21 Oct 2026 12:52:02 +0530 Subject: [PATCH 3/4] detect/flowbits: check for unsatisfiable dependencies Flowbits could be used across multiple signatures in a way that the final signature list may never reach a state such that all the flowbits rules could fire. e.g. cyclic dependencies among different flowbits on different signatures. Create a graph from the analyzer array and check if it results in a satisfiable path at runtime. Issue an error in case the dependencies cannot be met alongwith the information about the signatures involved in the complex chain ordering. Ticket: 7771 --- src/detect-flowbits.c | 106 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 105 insertions(+), 1 deletion(-) diff --git a/src/detect-flowbits.c b/src/detect-flowbits.c index 6c29906262..e387347c52 100644 --- a/src/detect-flowbits.c +++ b/src/detect-flowbits.c @@ -1,4 +1,4 @@ -/* Copyright (C) 2007-2025 Open Information Security Foundation +/* Copyright (C) 2007-2026 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 @@ -617,6 +617,107 @@ int DetectFlowbitsAnalyzeSignature(const Signature *s, struct FBAnalyzer *fba) return 0; } +static int CreateGraphFromFlowbitAnalyzer(void *graph, const struct FBAnalyzer *fba) +{ + // varnamestore ids start at 1 + for (uint32_t x = 1; x < fba->array_size; x++) { + const struct FBAnalyze *fa = &fba->array[x]; + for (uint32_t i = 0; i < fa->isset_iids_idx; i++) { + SCLogDebug("Getting or creating node for sid: %u", fa->isset_iids[i].sid); + int64_t to = + SCGetOrCreateNodeGraph(graph, fa->isset_iids[i].iid, fa->isset_iids[i].sid); + if (to < 0) { + SCLogError("Couldn't create or get a node in the graph for sid: %u", + fa->isset_iids[i].sid); + return -1; + } + SCLogDebug("added node: %" PRId64, to); + for (uint32_t y = 0; y < fa->set_iids_idx; y++) { + SCLogDebug("Getting or creating node for sid: %u", fa->set_iids[y].sid); + int64_t from = + SCGetOrCreateNodeGraph(graph, fa->set_iids[y].iid, fa->set_iids[y].sid); + if (from < 0) { + SCLogError("Couldn't create or get a node in the graph for sid: %u", + fa->set_iids[y].sid); + return -1; + } + SCLogDebug("added node: %" PRId64, from); + if (SCCreateNodeEdgeDirectedGraph( + graph, (uint32_t)from, (uint32_t)to, DETECT_FLOWBITS_CMD_SET, x) < 0) { + SCLogDebug( + "Flowbits signatures that are unsatisfiable at runtime found: %u, %u", + fa->isset_iids[i].sid, fa->set_iids[y].sid); + return -1; + } + } + } + } + + for (uint32_t x = 1; x < fba->array_size; x++) { + const struct FBAnalyze *fa = &fba->array[x]; + for (uint32_t i = 0; i < fa->isnotset_iids_idx; i++) { + int64_t to = SCGetOrCreateNodeGraph( + graph, fa->isnotset_iids[i].iid, fa->isnotset_iids[i].sid); + if (to < 0) { + SCLogError("Couldn't create or get a node in the graph for sid: %u", + fa->isnotset_iids[i].sid); + return -1; + } + SCLogDebug("added node: %" PRId64, to); + for (uint32_t y = 0; y < fa->unset_iids_idx; y++) { + int64_t from = + SCGetOrCreateNodeGraph(graph, fa->unset_iids[y].iid, fa->unset_iids[y].sid); + if (from < 0) { + SCLogError("Couldn't create or get a node in the graph for sid: %u", + fa->unset_iids[y].sid); + return -1; + } + SCLogDebug("added node: %" PRId64, from); + if (SCCreateNodeEdgeDirectedGraph(graph, (uint32_t)from, (uint32_t)to, + DETECT_FLOWBITS_CMD_UNSET, x) < 0) { + SCLogDebug( + "Flowbits signatures that are unsatisfiable at runtime found: %u, %u", + fa->isnotset_iids[i].sid, fa->unset_iids[y].sid); + return -1; + } + } + } + } + + return 0; +} + +/** + * \brief Check flowbit dependencies for issues + * + * Builds the flowbit dependency graph from the analyzer array and checks it + * for dependencies that cannot be met at runtime. + * + * \param fba filled flowbit analyzer array + * + * \retval 0 if no issues were found + * \retval -1 on error + */ +static int DetectFlowbitsCheckDependencies(const struct FBAnalyzer *fba) +{ + void *graph = SCCreateDirectedGraph(); + if (CreateGraphFromFlowbitAnalyzer(graph, fba) < 0) { + SCLogError("Could not create Flowbit Analyzer Graph"); + goto error; + } + if (SCCheckFlowbitDependencies(graph) < 0) { + goto error; + } + + SCFreeDirectedGraph(graph); + return 0; + +error: + SCLogError("Error resolving flowbit dependencies"); + SCFreeDirectedGraph(graph); + return -1; +} + int DetectFlowbitsAnalyze(DetectEngineCtx *de_ctx) { const uint32_t max_fb_id = de_ctx->max_fb_id; @@ -647,6 +748,9 @@ int DetectFlowbitsAnalyze(DetectEngineCtx *de_ctx) } } + /* check for flowbit dependencies that cannot be met at runtime */ + (void)DetectFlowbitsCheckDependencies(&fba); + /* walk array to see if all bits make sense */ for (uint32_t i = 0; i < array_size; i++) { const char *varname = VarNameStoreSetupLookup(i, VAR_TYPE_FLOW_BIT); From 0060e98be04b3403a67659a1c8f14ae69f5b2c48 Mon Sep 17 00:00:00 2001 From: Shivani Bhardwaj Date: Wed, 21 Oct 2026 13:08:01 +0530 Subject: [PATCH 4/4] [RFC] flowbits: do not consider isnotset -> unset a dependency --- rust/src/utils/flowbits_resolver.rs | 40 +++++++++++++++-------------- 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/rust/src/utils/flowbits_resolver.rs b/rust/src/utils/flowbits_resolver.rs index 885029799e..4331ff7e2b 100644 --- a/rust/src/utils/flowbits_resolver.rs +++ b/rust/src/utils/flowbits_resolver.rs @@ -24,6 +24,9 @@ use petgraph::Direction; use std::collections::{HashMap, HashSet}; use std::os::raw::c_void; +/// Must match DETECT_FLOWBITS_CMD_SET in src/detect-flowbits.h +const DETECT_FLOWBITS_CMD_SET: u8 = 0; + /// Special Graph Node storing flowbit or signature #[derive(Debug, Copy, Clone)] struct SCGNode { @@ -197,26 +200,25 @@ fn try_resolve_one_cycle(graph: &mut StableDiGraph) -> bool { /* store cycle edges to inspect their commands (weights) later */ edge_map.insert(edge_idx, cmd); } - /* Find if the cycle causing edges are made up of differing weights */ - let distinct_weights = edge_map.values().copied().collect::>().len(); - debug_validate_bug_on!(distinct_weights == 0); - if distinct_weights > 1 { - /* Find and remove the edge with highest weight (lowest priority) */ - if let Some((cur_e, _)) = edge_map.iter().max_by_key(|(_, &weight)| weight) { - graph.remove_edge(*cur_e); - return true; - } - } else { - /* Valid cycle with same weights -- can't resolve */ - let mut sids: Vec<_> = scc_set.iter().map(|a| graph[*a].sid).collect(); - sids.sort_unstable(); - SCLogError!( - "Cyclic dependency found between flowbits from signatures: {:?}", - sids - ); - return false; + debug_validate_bug_on!(edge_map.is_empty()); + /* An unset -> isnotset edge is not a hard dependency: isnotset already + * matches before the bit was ever set, so drop such an edge if the + * cycle has one */ + if let Some((cur_e, _)) = edge_map + .iter() + .find(|(_, &cmd)| cmd != DETECT_FLOWBITS_CMD_SET) + { + graph.remove_edge(*cur_e); + return true; } - break; + /* Valid cycle made of set edges only -- can't resolve */ + let mut sids: Vec<_> = scc_set.iter().map(|a| graph[*a].sid).collect(); + sids.sort_unstable(); + SCLogError!( + "Cyclic dependency found between flowbits from signatures: {:?}", + sids + ); + return false; } /* couldn't resolve */