Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
12 changes: 5 additions & 7 deletions crates/parapet-conformance/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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:
Expand All @@ -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

Expand Down
10 changes: 5 additions & 5 deletions crates/parapet-conformance/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()))
Expand Down
14 changes: 5 additions & 9 deletions crates/parapet-conformance/src/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -227,8 +226,8 @@ pub fn run(rules_dir: &str) -> Result<usize, String> {
}

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());
Expand All @@ -243,10 +242,7 @@ pub fn run(rules_dir: &str) -> Result<usize, String> {
};
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;
Expand Down
1 change: 1 addition & 0 deletions crates/parapet/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
6 changes: 3 additions & 3 deletions crates/parapet/src/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
69 changes: 48 additions & 21 deletions crates/parapet/src/matcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
})
}

Expand Down Expand Up @@ -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())
}
}
}
}
Expand Down Expand Up @@ -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", "<script>alert(1)</script>"));
assert!(matches("@detectXSS", "<img src=x onerror=alert(1)>"));
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]
Expand Down
Loading