From 78a64c55532899cf266264546878559055d13e0f Mon Sep 17 00:00:00 2001 From: Nicolas Dreno Date: Fri, 11 Sep 2026 15:49:54 +0200 Subject: [PATCH] Integrate libinjection: implement @detectSQLi and @detectXSS The two libinjection operators parsed but refused to compile, so any CRS rule using them was unenforceable. Wire in the libinjectionrs port (now differential-tested to zero divergence from the C library over its corpus) and implement both operators: - @detectSQLi classifies with detect_sqli and, when capture is requested, places the fingerprint in the first capture, as ModSecurity does. - @detectXSS classifies with detect_xss. The dependency is a git pin on barbacane-dev/libinjectionrs so downstream git consumers of parapet resolve it. Conformance: the operators compile now, so the two known refusals are gone, the curated `sqli tautology` case is caught by rule 942100 rather than attributed as a gap, and the operator-refusal gate's known set is empty (any future refusal is flagged). --- Cargo.lock | 23 +++++++++ Cargo.toml | 1 + crates/parapet-conformance/README.md | 12 ++--- crates/parapet-conformance/src/main.rs | 10 ++-- crates/parapet-conformance/src/run.rs | 14 ++---- crates/parapet/Cargo.toml | 1 + crates/parapet/src/engine.rs | 6 +-- crates/parapet/src/matcher.rs | 69 ++++++++++++++++++-------- 8 files changed, 91 insertions(+), 45 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6ba22c9..66739ea 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -11,6 +11,12 @@ dependencies = [ "memchr", ] +[[package]] +name = "bitflags" +version = "2.13.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3ded4057c258ba199e2d26386d3af3780957ecaee6c4ef4041c6b4b8b97c0b06" + [[package]] name = "block-buffer" version = "0.10.4" @@ -77,6 +83,16 @@ version = "0.2.189" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3eaf3ede3fee6db1a4c2ee091bf8a8b4dccdc6d17f656fb07896ee72867612f2" +[[package]] +name = "libinjectionrs" +version = "0.1.1" +source = "git+https://github.com/barbacane-dev/libinjectionrs?rev=7f35f8097e9dd06005334a0c8f50661ce544a8df#7f35f8097e9dd06005334a0c8f50661ce544a8df" +dependencies = [ + "bitflags", + "serde_json", + "smallvec", +] + [[package]] name = "memchr" version = "2.8.3" @@ -88,6 +104,7 @@ name = "parapet" version = "0.0.0" dependencies = [ "aho-corasick", + "libinjectionrs", "quick-xml", "regex", "serde", @@ -216,6 +233,12 @@ dependencies = [ "digest", ] +[[package]] +name = "smallvec" +version = "1.16.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b9be42f50aa861c555654aa3a37f52f4b1074bacf4e48fe0ef7fa584e80f1f0f" + [[package]] name = "syn" version = "3.0.5" diff --git a/Cargo.toml b/Cargo.toml index 5cf23bc..cb061b2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -10,6 +10,7 @@ repository = "https://github.com/barbacane-dev/parapet" authors = ["Parapet contributors"] [workspace.dependencies] +libinjectionrs = { git = "https://github.com/barbacane-dev/libinjectionrs", rev = "7f35f8097e9dd06005334a0c8f50661ce544a8df" } quick-xml = "0.42" regex = "1.12" regex-syntax = "0.8" diff --git a/crates/parapet-conformance/README.md b/crates/parapet-conformance/README.md index 57b1339..5bc5274 100644 --- a/crates/parapet-conformance/README.md +++ b/crates/parapet-conformance/README.md @@ -42,9 +42,8 @@ Every operator in a pinned CRS release must compile, including resolving the cargo run -p parapet-conformance -- operators coreruleset-4.9.0/rules ``` -Expected against v4.9.0: 18 distinct operators, 19 data files, 656 of 660 -operator instances compiling, and exactly two refusals (`@detectSQLi` and -`@detectXSS`, 4 rules). +Expected against v4.9.0: 18 distinct operators, 19 data files, all 660 +operator instances compiling, and no refusals. The refusal set is itself a gate. A newly refused operator means a rule silently stopped being enforceable; a refusal disappearing because it got @@ -63,7 +62,7 @@ Blocking must come from CRS rule 949110 on accumulated anomaly score, the way a real deployment blocks, not from an individual rule denying. The corpus carries benign requests too: a false positive is as much a defect as a miss. -Current state: 18 cases, **0 wrong verdicts**, 1 known gap. +Current state: 18 cases, **0 wrong verdicts**, no known gaps. Two of the corpus expectations are worth reading, because both were wrong when first written and looked like engine bugs: @@ -72,9 +71,8 @@ first written and looked like engine bugs: tagged `paranoia-level/2`, so CRS at the default level does not block it. Asserting the allow keeps a future change from silently raising the effective paranoia level. -- `sqli tautology` is a `known_gap`, attributed to rule 942100 needing - `@detectSQLi`. Attributed gaps still run and still print; they just do not - fail the build. +- `sqli tautology` is caught by rule 942100, the libinjection SQLi rule + (`@detectSQLi`). ## 5. Transformations against a reference implementation diff --git a/crates/parapet-conformance/src/main.rs b/crates/parapet-conformance/src/main.rs index ad814aa..53f0d23 100644 --- a/crates/parapet-conformance/src/main.rs +++ b/crates/parapet-conformance/src/main.rs @@ -265,11 +265,11 @@ fn operator_report(dir: &str) -> ExitCode { } } - // The refusal set is itself a gate. A newly refused operator means a rule - // silently stopped being enforceable, which is the thing this crate exists - // to make impossible; going the other way (a refusal disappearing because - // it got implemented) is fine. - const KNOWN_REFUSALS: &[&str] = &["detectSQLi", "detectXSS"]; + // The refusal set is itself a gate. Every operator CRS uses compiles, so + // the set is empty: any operator that refuses is a rule that silently + // stopped being enforceable, which is the thing this crate exists to make + // impossible. + const KNOWN_REFUSALS: &[&str] = &[]; let unexpected_refusals: Vec<&String> = refused .keys() .filter(|name| !KNOWN_REFUSALS.contains(&name.as_str())) diff --git a/crates/parapet-conformance/src/run.rs b/crates/parapet-conformance/src/run.rs index c83b485..eea394a 100644 --- a/crates/parapet-conformance/src/run.rs +++ b/crates/parapet-conformance/src/run.rs @@ -72,9 +72,8 @@ const CASES: &[Case] = &[ uri: "/item?id=1%27%20OR%20%271%27%3D%271", body: None, expect_block: true, - // Rule 942100 is the libinjection SQLi rule and this is its signature - // catch. It is one of the four rules that refuse to compile. - known_gap: Some("needs @detectSQLi (rule 942100)"), + // Caught by rule 942100, the libinjection SQLi rule (@detectSQLi). + known_gap: None, }, Case { name: "sqli union select", @@ -227,8 +226,8 @@ pub fn run(rules_dir: &str) -> Result { } let loader = DirDataLoader::new(rules_dir); - // compile_all, not compile: the two libinjection operators are known to - // refuse, and this tool measures progress rather than enforcing rules. + // compile_all, not compile: this tool measures progress and reports any + // rule that fails to compile rather than aborting on the first one. let (ruleset, compile_errors) = RuleSet::compile_all(&directives, &loader); println!("rules compiled : {}", ruleset.rule_count()); @@ -243,10 +242,7 @@ pub fn run(rules_dir: &str) -> Result { }; println!(" phase {phase} : {}", ruleset.rules_in_phase(p)); } - println!( - "rules refused : {} (known: the two libinjection operators)", - compile_errors.len() - ); + println!("rules refused : {}", compile_errors.len()); println!(); let mut wrong = 0usize; diff --git a/crates/parapet/Cargo.toml b/crates/parapet/Cargo.toml index 691708a..f2ed999 100644 --- a/crates/parapet/Cargo.toml +++ b/crates/parapet/Cargo.toml @@ -13,6 +13,7 @@ authors.workspace = true [dependencies] aho-corasick.workspace = true +libinjectionrs.workspace = true quick-xml.workspace = true regex.workspace = true serde = { workspace = true, optional = true } diff --git a/crates/parapet/src/engine.rs b/crates/parapet/src/engine.rs index 0b70e24..c1087df 100644 --- a/crates/parapet/src/engine.rs +++ b/crates/parapet/src/engine.rs @@ -592,9 +592,9 @@ mod tests { } #[test] - fn an_unimplemented_operator_is_a_compile_error() { - let err = compile_src(r#"SecRule ARGS "@detectSQLi" "id:1,phase:2,deny""#).unwrap_err(); - assert!(matches!(err, CompileError::Operator { .. })); + fn the_libinjection_operators_compile() { + assert!(compile_src(r#"SecRule ARGS "@detectSQLi" "id:1,phase:2,deny""#).is_ok()); + assert!(compile_src(r#"SecRule ARGS "@detectXSS" "id:2,phase:2,deny""#).is_ok()); } #[test] diff --git a/crates/parapet/src/matcher.rs b/crates/parapet/src/matcher.rs index a6a022c..ff019b3 100644 --- a/crates/parapet/src/matcher.rs +++ b/crates/parapet/src/matcher.rs @@ -226,6 +226,10 @@ pub enum CompiledOperator { ValidateUrlEncoding, /// `@unconditionalMatch` UnconditionalMatch, + /// `@detectSQLi`: libinjection SQL injection classifier. + DetectSqli, + /// `@detectXSS`: libinjection cross-site scripting classifier. + DetectXss, } impl CompiledOperator { @@ -307,18 +311,8 @@ impl CompiledOperator { Operator::ValidateUtf8Encoding => CompiledOperator::ValidateUtf8Encoding, Operator::ValidateUrlEncoding => CompiledOperator::ValidateUrlEncoding, Operator::UnconditionalMatch => CompiledOperator::UnconditionalMatch, - Operator::DetectSqli => { - return Err(OperatorCompileError::NotImplemented { - name: "detectSQLi", - detail: "needs a libinjection SQL classifier, which is not integrated yet", - }) - } - Operator::DetectXss => { - return Err(OperatorCompileError::NotImplemented { - name: "detectXSS", - detail: "needs a libinjection XSS classifier, which is not integrated yet", - }) - } + Operator::DetectSqli => CompiledOperator::DetectSqli, + Operator::DetectXss => CompiledOperator::DetectXss, }) } @@ -408,6 +402,29 @@ impl CompiledOperator { OperatorMatch::plain(!url_encoding_is_valid(subject)) } CompiledOperator::UnconditionalMatch => OperatorMatch::plain(true), + CompiledOperator::DetectSqli => { + let result = libinjectionrs::detect_sqli(subject); + if !result.is_injection() { + return OperatorMatch::plain(false); + } + if capture { + // ModSecurity places the fingerprint in the first capture. + let fp = result + .fingerprint + .as_ref() + .map(|f| f.as_str().as_bytes().to_vec()) + .unwrap_or_default(); + OperatorMatch { + matched: true, + captures: vec![fp], + } + } else { + OperatorMatch::plain(true) + } + } + CompiledOperator::DetectXss => { + OperatorMatch::plain(libinjectionrs::detect_xss(subject).is_injection()) + } } } } @@ -721,15 +738,25 @@ mod tests { } #[test] - fn the_two_libinjection_operators_refuse_to_compile() { - // They parse, so a rule set using them is well-formed, but compiling - // them to something that never matches would be a silent bypass. - for spec in ["@detectSQLi", "@detectXSS"] { - let op = Operator::parse(spec).unwrap(); - let err = CompiledOperator::compile(&op, &NoDataLoader) - .expect_err("must refuse until libinjection is integrated"); - assert!(matches!(err, OperatorCompileError::NotImplemented { .. })); - } + fn detect_sqli_classifies_with_libinjection() { + assert!(matches("@detectSQLi", "1' OR '1'='1")); + assert!(matches("@detectSQLi", "1 UNION SELECT password FROM users")); + assert!(!matches("@detectSQLi", "hello world")); + } + + #[test] + fn detect_xss_classifies_with_libinjection() { + assert!(matches("@detectXSS", "")); + assert!(matches("@detectXSS", "")); + assert!(!matches("@detectXSS", "hello world")); + } + + #[test] + fn detect_sqli_captures_the_fingerprint() { + let m = compile("@detectSQLi").evaluate(b"1' OR '1'='1", &EmptyContext, true); + assert!(m.matched); + // The first capture is libinjection's fingerprint, a non-empty token. + assert!(!m.captures.first().map(Vec::is_empty).unwrap_or(true)); } #[test]