From 494f91407096f461ded344845327760f589417cf Mon Sep 17 00:00:00 2001 From: Simon Friis Vindum Date: Fri, 7 Mar 2025 09:03:06 +0100 Subject: [PATCH 1/7] Rust: Add regular expression injection query --- .../hello-project/summary.expected | 2 +- .../hello-workspace/summary.cargo.expected | 2 +- .../summary.rust-project.expected | 2 +- .../codeql/rust/frameworks/regex.model.yml | 7 ++ .../regex/RegexInjectionExtensions.qll | 67 +++++++++++++++++++ .../security/regex/RegexInjectionQuery.qll | 31 +++++++++ .../security/CWE-730/RegexInjection.qhelp | 56 ++++++++++++++++ .../security/CWE-730/RegexInjection.ql | 20 ++++++ .../security/CWE-730/RegexInjectionBad.rs | 8 +++ .../security/CWE-730/RegexInjectionGood.rs | 9 +++ .../dataflow/local/DataFlowStep.expected | 1 + .../diagnostics/SummaryStats.expected | 2 +- .../security/CWE-730/RegexInjection.expected | 28 ++++++++ .../security/CWE-730/RegexInjection.qlref | 2 + .../CWE-730/RegexInjectionSink.expected | 2 + .../security/CWE-730/RegexInjectionSink.ql | 4 ++ .../test/query-tests/security/CWE-730/main.rs | 36 ++++++++++ .../query-tests/security/CWE-730/options.yml | 3 + 18 files changed, 278 insertions(+), 4 deletions(-) create mode 100644 rust/ql/lib/codeql/rust/frameworks/regex.model.yml create mode 100644 rust/ql/lib/codeql/rust/security/regex/RegexInjectionExtensions.qll create mode 100644 rust/ql/lib/codeql/rust/security/regex/RegexInjectionQuery.qll create mode 100644 rust/ql/src/queries/security/CWE-730/RegexInjection.qhelp create mode 100644 rust/ql/src/queries/security/CWE-730/RegexInjection.ql create mode 100644 rust/ql/src/queries/security/CWE-730/RegexInjectionBad.rs create mode 100644 rust/ql/src/queries/security/CWE-730/RegexInjectionGood.rs create mode 100644 rust/ql/test/query-tests/security/CWE-730/RegexInjection.expected create mode 100644 rust/ql/test/query-tests/security/CWE-730/RegexInjection.qlref create mode 100644 rust/ql/test/query-tests/security/CWE-730/RegexInjectionSink.expected create mode 100644 rust/ql/test/query-tests/security/CWE-730/RegexInjectionSink.ql create mode 100644 rust/ql/test/query-tests/security/CWE-730/main.rs create mode 100644 rust/ql/test/query-tests/security/CWE-730/options.yml diff --git a/rust/ql/integration-tests/hello-project/summary.expected b/rust/ql/integration-tests/hello-project/summary.expected index 2ffb1f4e34f7..e5a531c16bab 100644 --- a/rust/ql/integration-tests/hello-project/summary.expected +++ b/rust/ql/integration-tests/hello-project/summary.expected @@ -14,7 +14,7 @@ | Macro calls - resolved | 2 | | Macro calls - total | 2 | | Macro calls - unresolved | 0 | -| Taint edges - number of edges | 1670 | +| Taint edges - number of edges | 1671 | | Taint reach - nodes tainted | 0 | | Taint reach - per million nodes | 0 | | Taint sinks - cryptographic operations | 0 | diff --git a/rust/ql/integration-tests/hello-workspace/summary.cargo.expected b/rust/ql/integration-tests/hello-workspace/summary.cargo.expected index d08ce1a41166..084025a1bb9f 100644 --- a/rust/ql/integration-tests/hello-workspace/summary.cargo.expected +++ b/rust/ql/integration-tests/hello-workspace/summary.cargo.expected @@ -14,7 +14,7 @@ | Macro calls - resolved | 2 | | Macro calls - total | 2 | | Macro calls - unresolved | 0 | -| Taint edges - number of edges | 1670 | +| Taint edges - number of edges | 1671 | | Taint reach - nodes tainted | 0 | | Taint reach - per million nodes | 0 | | Taint sinks - cryptographic operations | 0 | diff --git a/rust/ql/integration-tests/hello-workspace/summary.rust-project.expected b/rust/ql/integration-tests/hello-workspace/summary.rust-project.expected index d08ce1a41166..084025a1bb9f 100644 --- a/rust/ql/integration-tests/hello-workspace/summary.rust-project.expected +++ b/rust/ql/integration-tests/hello-workspace/summary.rust-project.expected @@ -14,7 +14,7 @@ | Macro calls - resolved | 2 | | Macro calls - total | 2 | | Macro calls - unresolved | 0 | -| Taint edges - number of edges | 1670 | +| Taint edges - number of edges | 1671 | | Taint reach - nodes tainted | 0 | | Taint reach - per million nodes | 0 | | Taint sinks - cryptographic operations | 0 | diff --git a/rust/ql/lib/codeql/rust/frameworks/regex.model.yml b/rust/ql/lib/codeql/rust/frameworks/regex.model.yml new file mode 100644 index 000000000000..b645dc0b955c --- /dev/null +++ b/rust/ql/lib/codeql/rust/frameworks/regex.model.yml @@ -0,0 +1,7 @@ +# Models for the `regex` crate. +extensions: + - addsTo: + pack: codeql/rust-all + extensible: summaryModel + data: + - ["repo:https://github.com/rust-lang/regex:regex", "crate::escape", "Argument[0].Reference", "ReturnValue", "taint", "manual"] diff --git a/rust/ql/lib/codeql/rust/security/regex/RegexInjectionExtensions.qll b/rust/ql/lib/codeql/rust/security/regex/RegexInjectionExtensions.qll new file mode 100644 index 000000000000..7e886146c2cf --- /dev/null +++ b/rust/ql/lib/codeql/rust/security/regex/RegexInjectionExtensions.qll @@ -0,0 +1,67 @@ +/** + * Provides classes and predicates to reason about regular expression injection + * vulnerabilities. + */ + +private import codeql.util.Unit +private import rust +private import codeql.rust.dataflow.DataFlow +private import codeql.rust.controlflow.CfgNodes +private import codeql.rust.dataflow.FlowSink + +/** + * A data flow sink for regular expression injection vulnerabilities. + */ +abstract class RegexInjectionSink extends DataFlow::Node { } + +/** + * A barrier for regular expression injection vulnerabilities. + */ +abstract class RegexInjectionBarrier extends DataFlow::Node { } + +/** A sink for `a` in `Regex::new(a)` when `a` is not a literal. */ +private class NewRegexInjectionSink extends RegexInjectionSink { + NewRegexInjectionSink() { + exists(CallExprCfgNode call, PathExpr path | + path = call.getFunction().getExpr() and + path.getResolvedCrateOrigin() = "repo:https://github.com/rust-lang/regex:regex" and + path.getResolvedPath() = "::new" and + this.asExpr() = call.getArgument(0) and + not this.asExpr() instanceof LiteralExprCfgNode + ) + } +} + +private class MadRegexInjectionSink extends RegexInjectionSink { + MadRegexInjectionSink() { sinkNode(this, "regex-use") } +} + +/** + * A unit class for adding additional flow steps. + */ +class RegexInjectionAdditionalFlowStep extends Unit { + /** + * Holds if the step from `node1` to `node2` should be considered a flow + * step for paths related to regular expression injection vulnerabilities. + */ + abstract predicate step(DataFlow::Node node1, DataFlow::Node node2); +} + +/** + * An escape barrier for regular expression injection vulnerabilities. + */ +private class RegexInjectionDefaultBarrier extends RegexInjectionBarrier { + RegexInjectionDefaultBarrier() { + // A barrier is any call to a function named `escape`, in particular this + // makes calls to `regex::escape` a barrier. + this.asExpr() + .getExpr() + .(CallExpr) + .getFunction() + .(PathExpr) + .getPath() + .getPart() + .getNameRef() + .getText() = "escape" + } +} diff --git a/rust/ql/lib/codeql/rust/security/regex/RegexInjectionQuery.qll b/rust/ql/lib/codeql/rust/security/regex/RegexInjectionQuery.qll new file mode 100644 index 000000000000..387b8caae309 --- /dev/null +++ b/rust/ql/lib/codeql/rust/security/regex/RegexInjectionQuery.qll @@ -0,0 +1,31 @@ +/** + * Provides a taint-tracking configuration for detecting regular expression + * injection vulnerabilities. + */ + +private import rust +private import codeql.rust.dataflow.DataFlow +private import codeql.rust.dataflow.TaintTracking +private import codeql.rust.dataflow.FlowSource +private import codeql.rust.Concepts +private import codeql.rust.security.regex.RegexInjectionExtensions + +/** + * A taint configuration for detecting regular expression injection vulnerabilities. + */ +module RegexInjectionConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source instanceof ThreatModelSource } + + predicate isSink(DataFlow::Node sink) { sink instanceof RegexInjectionSink } + + predicate isBarrier(DataFlow::Node barrier) { barrier instanceof RegexInjectionBarrier } + + predicate isAdditionalFlowStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) { + any(RegexInjectionAdditionalFlowStep s).step(nodeFrom, nodeTo) + } +} + +/** + * Detect taint flow of tainted data that reaches a regular expression sink. + */ +module RegexInjectionFlow = TaintTracking::Global; diff --git a/rust/ql/src/queries/security/CWE-730/RegexInjection.qhelp b/rust/ql/src/queries/security/CWE-730/RegexInjection.qhelp new file mode 100644 index 000000000000..046b172d3e7a --- /dev/null +++ b/rust/ql/src/queries/security/CWE-730/RegexInjection.qhelp @@ -0,0 +1,56 @@ + + + + +

+Constructing a regular expression with unsanitized user input can be dangerous. +A malicious user may be able to modify the meaning of the expression causing it +to match unexpected strings and to construct large regular expressions by using +counted repetitions. +

+ + + +

+Before embedding user input into a regular expression, escape the input string +using a function such as regex::escape to +escape meta-characters that have special meaning. +

+

+If purposefully supporting user supplied regular expressions, then use RegexBuilder::size_limit +to limit the pattern size such that it is no larger than necessary. +

+ + + +

+The following example construct a regular expressions from the user input +key without escaping it first. +

+ + + +

+The regular expression is intended to match strings starting with +"property" such as "property:foo=bar". However, a +malicious user might inject the regular expression ".*^|key" and +unexpectedly cause strings such as "key=secret" to match. +

+

+If user input is used to construct a regular expression it should be escaped +first. This ensures that the user cannot insert characters that have special +meanings in regular expressions. +

+ + + + +
  • + regex crate documentation: Untrusted patterns +
    • +     +     diff --git a/rust/ql/src/queries/security/CWE-730/RegexInjection.ql b/rust/ql/src/queries/security/CWE-730/RegexInjection.ql new file mode 100644 index 000000000000..88a85eb48583 --- /dev/null +++ b/rust/ql/src/queries/security/CWE-730/RegexInjection.ql @@ -0,0 +1,20 @@ +/** + * @name Regular expression injection + * @description + * @kind path-problem + * @problem.severity error + * @security-severity 7.5 + * @precision high + * @id rust/regex-injection + * @tags security + * external/cwe/cwe-730 + * external/cwe/cwe-400 + */ + +private import codeql.rust.security.regex.RegexInjectionQuery +private import RegexInjectionFlow::PathGraph + +from RegexInjectionFlow::PathNode sourceNode, RegexInjectionFlow::PathNode sinkNode +where RegexInjectionFlow::flowPath(sourceNode, sinkNode) +select sinkNode.getNode(), sourceNode, sinkNode, + "This regular expression is constructed from a $@.", sourceNode.getNode(), "user-provided value" diff --git a/rust/ql/src/queries/security/CWE-730/RegexInjectionBad.rs b/rust/ql/src/queries/security/CWE-730/RegexInjectionBad.rs new file mode 100644 index 000000000000..5d5bd78becc6 --- /dev/null +++ b/rust/ql/src/queries/security/CWE-730/RegexInjectionBad.rs @@ -0,0 +1,8 @@ +use regex::Regex; + +fn get_value<'h>(key: &str, property: &'h str) -> Option<&'h str> { + // BAD: User provided `key` is interpolated into the regular expression. + let pattern = format!(r"^property:{key}=(.*)$"); + let re = Regex::new(&pattern).unwrap(); + re.captures(property)?.get(1).map(|m| m.as_str()) +} \ No newline at end of file diff --git a/rust/ql/src/queries/security/CWE-730/RegexInjectionGood.rs b/rust/ql/src/queries/security/CWE-730/RegexInjectionGood.rs new file mode 100644 index 000000000000..fa87435acdc0 --- /dev/null +++ b/rust/ql/src/queries/security/CWE-730/RegexInjectionGood.rs @@ -0,0 +1,9 @@ +use regex::{escape, Regex}; + +fn get_value<'h>(key: &str, property: &'h str) -> option<&'h str> { + // GOOD: User input is escaped before being used in the regular expression. + let escaped_key = escape(key); + let pattern = format!(r"^property:{escaped_key}=(.*)$"); + let re = regex::new(&pattern).unwrap(); + re.captures(property)?.get(1).map(|m| m.as_str()) +} \ No newline at end of file diff --git a/rust/ql/test/library-tests/dataflow/local/DataFlowStep.expected b/rust/ql/test/library-tests/dataflow/local/DataFlowStep.expected index e403311345cc..8a8c5988426a 100644 --- a/rust/ql/test/library-tests/dataflow/local/DataFlowStep.expected +++ b/rust/ql/test/library-tests/dataflow/local/DataFlowStep.expected @@ -2493,6 +2493,7 @@ readStep | file://:0:0:0:0 | [summary param] 0 in lang:std::_::crate::sys_common::ignore_notfound | Err | file://:0:0:0:0 | [summary] read: Argument[0].Field[crate::result::Result::Err(0)] in lang:std::_::crate::sys_common::ignore_notfound | | file://:0:0:0:0 | [summary param] 0 in lang:std::_::crate::thread::current::try_with_current | function return | file://:0:0:0:0 | [summary] read: Argument[0].ReturnValue in lang:std::_::crate::thread::current::try_with_current | | file://:0:0:0:0 | [summary param] 0 in lang:std::_::crate::thread::with_current_name | function return | file://:0:0:0:0 | [summary] read: Argument[0].ReturnValue in lang:std::_::crate::thread::with_current_name | +| file://:0:0:0:0 | [summary param] 0 in repo:https://github.com/rust-lang/regex:regex::_::crate::escape | &ref | file://:0:0:0:0 | [summary] read: Argument[0].Reference in repo:https://github.com/rust-lang/regex:regex::_::crate::escape | | file://:0:0:0:0 | [summary param] 1 in lang:alloc::_::::fold | function return | file://:0:0:0:0 | [summary] read: Argument[1].ReturnValue in lang:alloc::_::::fold | | file://:0:0:0:0 | [summary param] 1 in lang:alloc::_::crate::collections::btree::mem::replace | function return | file://:0:0:0:0 | [summary] read: Argument[1].ReturnValue in lang:alloc::_::crate::collections::btree::mem::replace | | file://:0:0:0:0 | [summary param] 1 in lang:alloc::_::crate::collections::btree::mem::take_mut | function return | file://:0:0:0:0 | [summary] read: Argument[1].ReturnValue in lang:alloc::_::crate::collections::btree::mem::take_mut | diff --git a/rust/ql/test/query-tests/diagnostics/SummaryStats.expected b/rust/ql/test/query-tests/diagnostics/SummaryStats.expected index 5249eae29b1f..7c7c2dd1961b 100644 --- a/rust/ql/test/query-tests/diagnostics/SummaryStats.expected +++ b/rust/ql/test/query-tests/diagnostics/SummaryStats.expected @@ -14,7 +14,7 @@ | Macro calls - resolved | 8 | | Macro calls - total | 9 | | Macro calls - unresolved | 1 | -| Taint edges - number of edges | 1670 | +| Taint edges - number of edges | 1671 | | Taint reach - nodes tainted | 0 | | Taint reach - per million nodes | 0 | | Taint sinks - cryptographic operations | 0 | diff --git a/rust/ql/test/query-tests/security/CWE-730/RegexInjection.expected b/rust/ql/test/query-tests/security/CWE-730/RegexInjection.expected new file mode 100644 index 000000000000..fd7ded696334 --- /dev/null +++ b/rust/ql/test/query-tests/security/CWE-730/RegexInjection.expected @@ -0,0 +1,28 @@ +#select +| main.rs:6:25:6:30 | ®ex | main.rs:4:20:4:32 | ...::var | main.rs:6:25:6:30 | ®ex | This regular expression is constructed from a $@. | main.rs:4:20:4:32 | ...::var | user-provided value | +edges +| main.rs:4:9:4:16 | username | main.rs:5:25:5:44 | MacroExpr | provenance | | +| main.rs:4:20:4:32 | ...::var | main.rs:4:20:4:40 | ...::var(...) [Ok] | provenance | Src:MaD:60 | +| main.rs:4:20:4:40 | ...::var(...) [Ok] | main.rs:4:20:4:66 | ... .unwrap_or(...) | provenance | MaD:1573 | +| main.rs:4:20:4:66 | ... .unwrap_or(...) | main.rs:4:9:4:16 | username | provenance | | +| main.rs:5:9:5:13 | regex | main.rs:6:26:6:30 | regex | provenance | | +| main.rs:5:17:5:45 | res | main.rs:5:25:5:44 | { ... } | provenance | | +| main.rs:5:25:5:44 | ...::format(...) | main.rs:5:17:5:45 | res | provenance | | +| main.rs:5:25:5:44 | ...::must_use(...) | main.rs:5:9:5:13 | regex | provenance | | +| main.rs:5:25:5:44 | MacroExpr | main.rs:5:25:5:44 | ...::format(...) | provenance | MaD:64 | +| main.rs:5:25:5:44 | { ... } | main.rs:5:25:5:44 | ...::must_use(...) | provenance | MaD:2996 | +| main.rs:6:26:6:30 | regex | main.rs:6:25:6:30 | ®ex | provenance | | +nodes +| main.rs:4:9:4:16 | username | semmle.label | username | +| main.rs:4:20:4:32 | ...::var | semmle.label | ...::var | +| main.rs:4:20:4:40 | ...::var(...) [Ok] | semmle.label | ...::var(...) [Ok] | +| main.rs:4:20:4:66 | ... .unwrap_or(...) | semmle.label | ... .unwrap_or(...) | +| main.rs:5:9:5:13 | regex | semmle.label | regex | +| main.rs:5:17:5:45 | res | semmle.label | res | +| main.rs:5:25:5:44 | ...::format(...) | semmle.label | ...::format(...) | +| main.rs:5:25:5:44 | ...::must_use(...) | semmle.label | ...::must_use(...) | +| main.rs:5:25:5:44 | MacroExpr | semmle.label | MacroExpr | +| main.rs:5:25:5:44 | { ... } | semmle.label | { ... } | +| main.rs:6:25:6:30 | ®ex | semmle.label | ®ex | +| main.rs:6:26:6:30 | regex | semmle.label | regex | +subpaths diff --git a/rust/ql/test/query-tests/security/CWE-730/RegexInjection.qlref b/rust/ql/test/query-tests/security/CWE-730/RegexInjection.qlref new file mode 100644 index 000000000000..d62d882f992d --- /dev/null +++ b/rust/ql/test/query-tests/security/CWE-730/RegexInjection.qlref @@ -0,0 +1,2 @@ +query: queries/security/CWE-730/RegexInjection.ql +postprocess: utils/test/InlineExpectationsTestQuery.ql diff --git a/rust/ql/test/query-tests/security/CWE-730/RegexInjectionSink.expected b/rust/ql/test/query-tests/security/CWE-730/RegexInjectionSink.expected new file mode 100644 index 000000000000..4ea46a385870 --- /dev/null +++ b/rust/ql/test/query-tests/security/CWE-730/RegexInjectionSink.expected @@ -0,0 +1,2 @@ +| main.rs:6:25:6:30 | ®ex | +| main.rs:14:25:14:30 | ®ex | diff --git a/rust/ql/test/query-tests/security/CWE-730/RegexInjectionSink.ql b/rust/ql/test/query-tests/security/CWE-730/RegexInjectionSink.ql new file mode 100644 index 000000000000..0723ec5bb5b7 --- /dev/null +++ b/rust/ql/test/query-tests/security/CWE-730/RegexInjectionSink.ql @@ -0,0 +1,4 @@ +private import codeql.rust.dataflow.DataFlow +private import codeql.rust.security.regex.RegexInjectionExtensions + +query predicate regexInjectionSink(DataFlow::Node node) { node instanceof RegexInjectionSink } diff --git a/rust/ql/test/query-tests/security/CWE-730/main.rs b/rust/ql/test/query-tests/security/CWE-730/main.rs new file mode 100644 index 000000000000..a03e5d4b4942 --- /dev/null +++ b/rust/ql/test/query-tests/security/CWE-730/main.rs @@ -0,0 +1,36 @@ +use regex::Regex; + +fn simple_bad(hay: &str) -> Option { + let username = std::env::var("USER").unwrap_or("".to_string()); // $ Source=env + let regex = format!("foo{}bar", username); + let re = Regex::new(®ex).unwrap(); // $ Alert[rust/regex-injection]=env + Some(re.is_match(hay)) +} + +fn simple_good(hay: &str) -> Option { + let username = std::env::var("USER").unwrap_or("".to_string()); + let escaped = regex::escape(&username); + let regex = format!("foo{}bar", escaped); + let re = Regex::new(®ex).unwrap(); + Some(re.is_match(hay)) +} + +fn not_a_sink_literal() -> Option { + let username = std::env::var("USER").unwrap_or("".to_string()); + let re = Regex::new("literal string").unwrap(); + Some(re.is_match(&username)) +} + +fn not_a_sink_raw_literal() -> Option { + let username = std::env::var("USER").unwrap_or("".to_string()); + let re = Regex::new(r"literal string").unwrap(); + Some(re.is_match(&username)) +} + +fn main() { + let hay = "a string"; + simple_bad(hay); + simple_good(hay); + not_a_sink_literal(); + not_a_sink_raw_literal(); +} diff --git a/rust/ql/test/query-tests/security/CWE-730/options.yml b/rust/ql/test/query-tests/security/CWE-730/options.yml new file mode 100644 index 000000000000..8d484623101f --- /dev/null +++ b/rust/ql/test/query-tests/security/CWE-730/options.yml @@ -0,0 +1,3 @@ +qltest_cargo_check: true +qltest_dependencies: + - regex = { version = "1.11.1" } From 179ea041f45967a1c570f305d88802d8d36abe64 Mon Sep 17 00:00:00 2001 From: Simon Friis Vindum Date: Mon, 10 Mar 2025 09:09:13 +0100 Subject: [PATCH 2/7] Rust: Merge query implementation into one file --- .../security/regex/RegexInjectionQuery.qll | 31 ------------------- .../security/CWE-730/RegexInjection.ql | 27 +++++++++++++++- 2 files changed, 26 insertions(+), 32 deletions(-) delete mode 100644 rust/ql/lib/codeql/rust/security/regex/RegexInjectionQuery.qll diff --git a/rust/ql/lib/codeql/rust/security/regex/RegexInjectionQuery.qll b/rust/ql/lib/codeql/rust/security/regex/RegexInjectionQuery.qll deleted file mode 100644 index 387b8caae309..000000000000 --- a/rust/ql/lib/codeql/rust/security/regex/RegexInjectionQuery.qll +++ /dev/null @@ -1,31 +0,0 @@ -/** - * Provides a taint-tracking configuration for detecting regular expression - * injection vulnerabilities. - */ - -private import rust -private import codeql.rust.dataflow.DataFlow -private import codeql.rust.dataflow.TaintTracking -private import codeql.rust.dataflow.FlowSource -private import codeql.rust.Concepts -private import codeql.rust.security.regex.RegexInjectionExtensions - -/** - * A taint configuration for detecting regular expression injection vulnerabilities. - */ -module RegexInjectionConfig implements DataFlow::ConfigSig { - predicate isSource(DataFlow::Node source) { source instanceof ThreatModelSource } - - predicate isSink(DataFlow::Node sink) { sink instanceof RegexInjectionSink } - - predicate isBarrier(DataFlow::Node barrier) { barrier instanceof RegexInjectionBarrier } - - predicate isAdditionalFlowStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) { - any(RegexInjectionAdditionalFlowStep s).step(nodeFrom, nodeTo) - } -} - -/** - * Detect taint flow of tainted data that reaches a regular expression sink. - */ -module RegexInjectionFlow = TaintTracking::Global; diff --git a/rust/ql/src/queries/security/CWE-730/RegexInjection.ql b/rust/ql/src/queries/security/CWE-730/RegexInjection.ql index 88a85eb48583..64cfff45cb09 100644 --- a/rust/ql/src/queries/security/CWE-730/RegexInjection.ql +++ b/rust/ql/src/queries/security/CWE-730/RegexInjection.ql @@ -11,7 +11,32 @@ * external/cwe/cwe-400 */ -private import codeql.rust.security.regex.RegexInjectionQuery +private import rust +private import codeql.rust.dataflow.DataFlow +private import codeql.rust.dataflow.TaintTracking +private import codeql.rust.Concepts +private import codeql.rust.security.regex.RegexInjectionExtensions + +/** + * A taint configuration for detecting regular expression injection vulnerabilities. + */ +module RegexInjectionConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source instanceof ThreatModelSource } + + predicate isSink(DataFlow::Node sink) { sink instanceof RegexInjectionSink } + + predicate isBarrier(DataFlow::Node barrier) { barrier instanceof RegexInjectionBarrier } + + predicate isAdditionalFlowStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) { + any(RegexInjectionAdditionalFlowStep s).step(nodeFrom, nodeTo) + } +} + +/** + * Detect taint flow of tainted data that reaches a regular expression sink. + */ +module RegexInjectionFlow = TaintTracking::Global; + private import RegexInjectionFlow::PathGraph from RegexInjectionFlow::PathNode sourceNode, RegexInjectionFlow::PathNode sinkNode From 344fea21283f09c7d51323d6cd9efb90647b1318 Mon Sep 17 00:00:00 2001 From: Simon Friis Vindum Date: Mon, 10 Mar 2025 13:23:20 +0100 Subject: [PATCH 3/7] Rust: Enable local threat models in tests and use active threat models for regex query --- rust/ql/src/queries/security/CWE-730/RegexInjection.ql | 2 +- rust/ql/test/default-threat-models.model.yml | 6 ++++++ rust/ql/test/qlpack.yml | 2 ++ 3 files changed, 9 insertions(+), 1 deletion(-) create mode 100644 rust/ql/test/default-threat-models.model.yml diff --git a/rust/ql/src/queries/security/CWE-730/RegexInjection.ql b/rust/ql/src/queries/security/CWE-730/RegexInjection.ql index 64cfff45cb09..0d50d8ead6d8 100644 --- a/rust/ql/src/queries/security/CWE-730/RegexInjection.ql +++ b/rust/ql/src/queries/security/CWE-730/RegexInjection.ql @@ -21,7 +21,7 @@ private import codeql.rust.security.regex.RegexInjectionExtensions * A taint configuration for detecting regular expression injection vulnerabilities. */ module RegexInjectionConfig implements DataFlow::ConfigSig { - predicate isSource(DataFlow::Node source) { source instanceof ThreatModelSource } + predicate isSource(DataFlow::Node source) { source instanceof ActiveThreatModelSource } predicate isSink(DataFlow::Node sink) { sink instanceof RegexInjectionSink } diff --git a/rust/ql/test/default-threat-models.model.yml b/rust/ql/test/default-threat-models.model.yml new file mode 100644 index 000000000000..63507f477386 --- /dev/null +++ b/rust/ql/test/default-threat-models.model.yml @@ -0,0 +1,6 @@ +extensions: + - addsTo: + pack: codeql/threat-models + extensible: threatModelConfiguration + data: + - ["local", true, 0] diff --git a/rust/ql/test/qlpack.yml b/rust/ql/test/qlpack.yml index 7428ca33bed7..1bf759f71e09 100644 --- a/rust/ql/test/qlpack.yml +++ b/rust/ql/test/qlpack.yml @@ -6,3 +6,5 @@ dependencies: extractor: rust tests: . warnOnImplicitThis: true +dataExtensions: + - default-threat-models.model.yml From 0e965f7616a5bbd1a8df76c00a7648b58925c672 Mon Sep 17 00:00:00 2001 From: Simon Friis Vindum Date: Mon, 10 Mar 2025 14:39:37 +0100 Subject: [PATCH 4/7] Rust: Accept changes --- .../dataflow/sources/TaintSources.expected | 32 ++++---- .../security/CWE-089/SqlInjection.expected | 76 +++++++++++++------ .../test/query-tests/security/CWE-089/sqlx.rs | 6 +- 3 files changed, 71 insertions(+), 43 deletions(-) diff --git a/rust/ql/test/library-tests/dataflow/sources/TaintSources.expected b/rust/ql/test/library-tests/dataflow/sources/TaintSources.expected index 603ba0d76058..8448ee783720 100644 --- a/rust/ql/test/library-tests/dataflow/sources/TaintSources.expected +++ b/rust/ql/test/library-tests/dataflow/sources/TaintSources.expected @@ -1,18 +1,18 @@ -| test.rs:8:10:8:22 | ...::var | Flow source 'EnvironmentSource' of type environment. | -| test.rs:9:10:9:25 | ...::var_os | Flow source 'EnvironmentSource' of type environment. | -| test.rs:11:16:11:28 | ...::var | Flow source 'EnvironmentSource' of type environment. | -| test.rs:12:16:12:31 | ...::var_os | Flow source 'EnvironmentSource' of type environment. | -| test.rs:17:25:17:38 | ...::vars | Flow source 'EnvironmentSource' of type environment. | -| test.rs:22:25:22:41 | ...::vars_os | Flow source 'EnvironmentSource' of type environment. | -| test.rs:29:29:29:42 | ...::args | Flow source 'CommandLineArgs' of type commandargs. | -| test.rs:32:16:32:29 | ...::args | Flow source 'CommandLineArgs' of type commandargs. | -| test.rs:33:16:33:32 | ...::args_os | Flow source 'CommandLineArgs' of type commandargs. | -| test.rs:34:16:34:29 | ...::args | Flow source 'CommandLineArgs' of type commandargs. | -| test.rs:42:16:42:29 | ...::args | Flow source 'CommandLineArgs' of type commandargs. | -| test.rs:46:16:46:32 | ...::args_os | Flow source 'CommandLineArgs' of type commandargs. | -| test.rs:52:15:52:35 | ...::current_dir | Flow source 'CommandLineArgs' of type commandargs. | -| test.rs:53:15:53:35 | ...::current_exe | Flow source 'CommandLineArgs' of type commandargs. | -| test.rs:54:16:54:33 | ...::home_dir | Flow source 'CommandLineArgs' of type commandargs. | +| test.rs:8:10:8:22 | ...::var | Flow source 'EnvironmentSource' of type environment (DEFAULT). | +| test.rs:9:10:9:25 | ...::var_os | Flow source 'EnvironmentSource' of type environment (DEFAULT). | +| test.rs:11:16:11:28 | ...::var | Flow source 'EnvironmentSource' of type environment (DEFAULT). | +| test.rs:12:16:12:31 | ...::var_os | Flow source 'EnvironmentSource' of type environment (DEFAULT). | +| test.rs:17:25:17:38 | ...::vars | Flow source 'EnvironmentSource' of type environment (DEFAULT). | +| test.rs:22:25:22:41 | ...::vars_os | Flow source 'EnvironmentSource' of type environment (DEFAULT). | +| test.rs:29:29:29:42 | ...::args | Flow source 'CommandLineArgs' of type commandargs (DEFAULT). | +| test.rs:32:16:32:29 | ...::args | Flow source 'CommandLineArgs' of type commandargs (DEFAULT). | +| test.rs:33:16:33:32 | ...::args_os | Flow source 'CommandLineArgs' of type commandargs (DEFAULT). | +| test.rs:34:16:34:29 | ...::args | Flow source 'CommandLineArgs' of type commandargs (DEFAULT). | +| test.rs:42:16:42:29 | ...::args | Flow source 'CommandLineArgs' of type commandargs (DEFAULT). | +| test.rs:46:16:46:32 | ...::args_os | Flow source 'CommandLineArgs' of type commandargs (DEFAULT). | +| test.rs:52:15:52:35 | ...::current_dir | Flow source 'CommandLineArgs' of type commandargs (DEFAULT). | +| test.rs:53:15:53:35 | ...::current_exe | Flow source 'CommandLineArgs' of type commandargs (DEFAULT). | +| test.rs:54:16:54:33 | ...::home_dir | Flow source 'CommandLineArgs' of type commandargs (DEFAULT). | | test.rs:62:26:62:47 | ...::get | Flow source 'RemoteSource' of type remote (DEFAULT). | | test.rs:65:26:65:47 | ...::get | Flow source 'RemoteSource' of type remote (DEFAULT). | | test.rs:68:26:68:47 | ...::get | Flow source 'RemoteSource' of type remote (DEFAULT). | @@ -22,4 +22,4 @@ | test.rs:80:24:80:35 | ...::get | Flow source 'RemoteSource' of type remote (DEFAULT). | | test.rs:112:35:112:46 | send_request | Flow source 'RemoteSource' of type remote (DEFAULT). | | test.rs:119:31:119:42 | send_request | Flow source 'RemoteSource' of type remote (DEFAULT). | -| test.rs:203:16:203:29 | ...::args | Flow source 'CommandLineArgs' of type commandargs. | +| test.rs:203:16:203:29 | ...::args | Flow source 'CommandLineArgs' of type commandargs (DEFAULT). | diff --git a/rust/ql/test/query-tests/security/CWE-089/SqlInjection.expected b/rust/ql/test/query-tests/security/CWE-089/SqlInjection.expected index 6b04453ec9ae..0d6f031a18f0 100644 --- a/rust/ql/test/query-tests/security/CWE-089/SqlInjection.expected +++ b/rust/ql/test/query-tests/security/CWE-089/SqlInjection.expected @@ -1,52 +1,73 @@ #select | sqlx.rs:62:26:62:46 | safe_query_3.as_str(...) | sqlx.rs:48:25:48:46 | ...::get | sqlx.rs:62:26:62:46 | safe_query_3.as_str(...) | This query depends on a $@. | sqlx.rs:48:25:48:46 | ...::get | user-provided value | +| sqlx.rs:63:26:63:48 | unsafe_query_1.as_str(...) | sqlx.rs:47:22:47:35 | ...::args | sqlx.rs:63:26:63:48 | unsafe_query_1.as_str(...) | This query depends on a $@. | sqlx.rs:47:22:47:35 | ...::args | user-provided value | | sqlx.rs:65:30:65:52 | unsafe_query_2.as_str(...) | sqlx.rs:48:25:48:46 | ...::get | sqlx.rs:65:30:65:52 | unsafe_query_2.as_str(...) | This query depends on a $@. | sqlx.rs:48:25:48:46 | ...::get | user-provided value | | sqlx.rs:67:30:67:52 | unsafe_query_4.as_str(...) | sqlx.rs:48:25:48:46 | ...::get | sqlx.rs:67:30:67:52 | unsafe_query_4.as_str(...) | This query depends on a $@. | sqlx.rs:48:25:48:46 | ...::get | user-provided value | | sqlx.rs:73:25:73:45 | safe_query_3.as_str(...) | sqlx.rs:48:25:48:46 | ...::get | sqlx.rs:73:25:73:45 | safe_query_3.as_str(...) | This query depends on a $@. | sqlx.rs:48:25:48:46 | ...::get | user-provided value | +| sqlx.rs:74:25:74:47 | unsafe_query_1.as_str(...) | sqlx.rs:47:22:47:35 | ...::args | sqlx.rs:74:25:74:47 | unsafe_query_1.as_str(...) | This query depends on a $@. | sqlx.rs:47:22:47:35 | ...::args | user-provided value | | sqlx.rs:76:29:76:51 | unsafe_query_2.as_str(...) | sqlx.rs:48:25:48:46 | ...::get | sqlx.rs:76:29:76:51 | unsafe_query_2.as_str(...) | This query depends on a $@. | sqlx.rs:48:25:48:46 | ...::get | user-provided value | | sqlx.rs:78:29:78:51 | unsafe_query_4.as_str(...) | sqlx.rs:48:25:48:46 | ...::get | sqlx.rs:78:29:78:51 | unsafe_query_4.as_str(...) | This query depends on a $@. | sqlx.rs:48:25:48:46 | ...::get | user-provided value | edges -| sqlx.rs:48:9:48:21 | remote_string | sqlx.rs:49:25:49:52 | remote_string.parse(...) [Ok] | provenance | MaD:6 | +| sqlx.rs:47:9:47:18 | arg_string | sqlx.rs:53:27:53:36 | arg_string | provenance | | +| sqlx.rs:47:22:47:35 | ...::args | sqlx.rs:47:22:47:37 | ...::args(...) [element] | provenance | Src:MaD:1 | +| sqlx.rs:47:22:47:37 | ...::args(...) [element] | sqlx.rs:47:22:47:44 | ... .nth(...) [Some] | provenance | MaD:10 | +| sqlx.rs:47:22:47:44 | ... .nth(...) [Some] | sqlx.rs:47:22:47:77 | ... .unwrap_or(...) | provenance | MaD:5 | +| sqlx.rs:47:22:47:77 | ... .unwrap_or(...) | sqlx.rs:47:9:47:18 | arg_string | provenance | | +| sqlx.rs:48:9:48:21 | remote_string | sqlx.rs:49:25:49:52 | remote_string.parse(...) [Ok] | provenance | MaD:8 | | sqlx.rs:48:9:48:21 | remote_string | sqlx.rs:54:27:54:39 | remote_string | provenance | | | sqlx.rs:48:9:48:21 | remote_string | sqlx.rs:56:34:56:89 | MacroExpr | provenance | | -| sqlx.rs:48:25:48:46 | ...::get | sqlx.rs:48:25:48:69 | ...::get(...) [Ok] | provenance | Src:MaD:1 | -| sqlx.rs:48:25:48:69 | ...::get(...) [Ok] | sqlx.rs:48:25:48:78 | ... .unwrap(...) | provenance | MaD:4 | -| sqlx.rs:48:25:48:78 | ... .unwrap(...) | sqlx.rs:48:25:48:85 | ... .text(...) [Ok] | provenance | MaD:8 | -| sqlx.rs:48:25:48:85 | ... .text(...) [Ok] | sqlx.rs:48:25:48:118 | ... .unwrap_or(...) | provenance | MaD:5 | +| sqlx.rs:48:25:48:46 | ...::get | sqlx.rs:48:25:48:69 | ...::get(...) [Ok] | provenance | Src:MaD:2 | +| sqlx.rs:48:25:48:69 | ...::get(...) [Ok] | sqlx.rs:48:25:48:78 | ... .unwrap(...) | provenance | MaD:6 | +| sqlx.rs:48:25:48:78 | ... .unwrap(...) | sqlx.rs:48:25:48:85 | ... .text(...) [Ok] | provenance | MaD:11 | +| sqlx.rs:48:25:48:85 | ... .text(...) [Ok] | sqlx.rs:48:25:48:118 | ... .unwrap_or(...) | provenance | MaD:7 | | sqlx.rs:48:25:48:118 | ... .unwrap_or(...) | sqlx.rs:48:9:48:21 | remote_string | provenance | | | sqlx.rs:49:9:49:21 | remote_number | sqlx.rs:52:32:52:87 | MacroExpr | provenance | | -| sqlx.rs:49:25:49:52 | remote_string.parse(...) [Ok] | sqlx.rs:49:25:49:65 | ... .unwrap_or(...) | provenance | MaD:5 | +| sqlx.rs:49:25:49:52 | remote_string.parse(...) [Ok] | sqlx.rs:49:25:49:65 | ... .unwrap_or(...) | provenance | MaD:7 | | sqlx.rs:49:25:49:65 | ... .unwrap_or(...) | sqlx.rs:49:9:49:21 | remote_number | provenance | | -| sqlx.rs:52:9:52:20 | safe_query_3 | sqlx.rs:62:26:62:46 | safe_query_3.as_str(...) | provenance | MaD:2 | -| sqlx.rs:52:9:52:20 | safe_query_3 | sqlx.rs:73:25:73:45 | safe_query_3.as_str(...) | provenance | MaD:2 | +| sqlx.rs:52:9:52:20 | safe_query_3 | sqlx.rs:62:26:62:46 | safe_query_3.as_str(...) | provenance | MaD:3 | +| sqlx.rs:52:9:52:20 | safe_query_3 | sqlx.rs:73:25:73:45 | safe_query_3.as_str(...) | provenance | MaD:3 | | sqlx.rs:52:24:52:88 | res | sqlx.rs:52:32:52:87 | { ... } | provenance | | | sqlx.rs:52:32:52:87 | ...::format(...) | sqlx.rs:52:24:52:88 | res | provenance | | | sqlx.rs:52:32:52:87 | ...::must_use(...) | sqlx.rs:52:9:52:20 | safe_query_3 | provenance | | -| sqlx.rs:52:32:52:87 | MacroExpr | sqlx.rs:52:32:52:87 | ...::format(...) | provenance | MaD:3 | -| sqlx.rs:52:32:52:87 | { ... } | sqlx.rs:52:32:52:87 | ...::must_use(...) | provenance | MaD:7 | +| sqlx.rs:52:32:52:87 | MacroExpr | sqlx.rs:52:32:52:87 | ...::format(...) | provenance | MaD:4 | +| sqlx.rs:52:32:52:87 | { ... } | sqlx.rs:52:32:52:87 | ...::must_use(...) | provenance | MaD:9 | +| sqlx.rs:53:9:53:22 | unsafe_query_1 [&ref] | sqlx.rs:63:26:63:39 | unsafe_query_1 [&ref] | provenance | | +| sqlx.rs:53:9:53:22 | unsafe_query_1 [&ref] | sqlx.rs:74:25:74:38 | unsafe_query_1 [&ref] | provenance | | +| sqlx.rs:53:26:53:36 | &arg_string [&ref] | sqlx.rs:53:9:53:22 | unsafe_query_1 [&ref] | provenance | | +| sqlx.rs:53:27:53:36 | arg_string | sqlx.rs:53:26:53:36 | &arg_string [&ref] | provenance | | | sqlx.rs:54:9:54:22 | unsafe_query_2 [&ref] | sqlx.rs:65:30:65:43 | unsafe_query_2 [&ref] | provenance | | | sqlx.rs:54:9:54:22 | unsafe_query_2 [&ref] | sqlx.rs:76:29:76:42 | unsafe_query_2 [&ref] | provenance | | | sqlx.rs:54:26:54:39 | &remote_string [&ref] | sqlx.rs:54:9:54:22 | unsafe_query_2 [&ref] | provenance | | | sqlx.rs:54:27:54:39 | remote_string | sqlx.rs:54:26:54:39 | &remote_string [&ref] | provenance | | -| sqlx.rs:56:9:56:22 | unsafe_query_4 | sqlx.rs:67:30:67:52 | unsafe_query_4.as_str(...) | provenance | MaD:2 | -| sqlx.rs:56:9:56:22 | unsafe_query_4 | sqlx.rs:78:29:78:51 | unsafe_query_4.as_str(...) | provenance | MaD:2 | +| sqlx.rs:56:9:56:22 | unsafe_query_4 | sqlx.rs:67:30:67:52 | unsafe_query_4.as_str(...) | provenance | MaD:3 | +| sqlx.rs:56:9:56:22 | unsafe_query_4 | sqlx.rs:78:29:78:51 | unsafe_query_4.as_str(...) | provenance | MaD:3 | | sqlx.rs:56:26:56:90 | res | sqlx.rs:56:34:56:89 | { ... } | provenance | | | sqlx.rs:56:34:56:89 | ...::format(...) | sqlx.rs:56:26:56:90 | res | provenance | | | sqlx.rs:56:34:56:89 | ...::must_use(...) | sqlx.rs:56:9:56:22 | unsafe_query_4 | provenance | | -| sqlx.rs:56:34:56:89 | MacroExpr | sqlx.rs:56:34:56:89 | ...::format(...) | provenance | MaD:3 | -| sqlx.rs:56:34:56:89 | { ... } | sqlx.rs:56:34:56:89 | ...::must_use(...) | provenance | MaD:7 | -| sqlx.rs:65:30:65:43 | unsafe_query_2 [&ref] | sqlx.rs:65:30:65:52 | unsafe_query_2.as_str(...) | provenance | MaD:2 | -| sqlx.rs:76:29:76:42 | unsafe_query_2 [&ref] | sqlx.rs:76:29:76:51 | unsafe_query_2.as_str(...) | provenance | MaD:2 | +| sqlx.rs:56:34:56:89 | MacroExpr | sqlx.rs:56:34:56:89 | ...::format(...) | provenance | MaD:4 | +| sqlx.rs:56:34:56:89 | { ... } | sqlx.rs:56:34:56:89 | ...::must_use(...) | provenance | MaD:9 | +| sqlx.rs:63:26:63:39 | unsafe_query_1 [&ref] | sqlx.rs:63:26:63:48 | unsafe_query_1.as_str(...) | provenance | MaD:3 | +| sqlx.rs:65:30:65:43 | unsafe_query_2 [&ref] | sqlx.rs:65:30:65:52 | unsafe_query_2.as_str(...) | provenance | MaD:3 | +| sqlx.rs:74:25:74:38 | unsafe_query_1 [&ref] | sqlx.rs:74:25:74:47 | unsafe_query_1.as_str(...) | provenance | MaD:3 | +| sqlx.rs:76:29:76:42 | unsafe_query_2 [&ref] | sqlx.rs:76:29:76:51 | unsafe_query_2.as_str(...) | provenance | MaD:3 | models -| 1 | Source: repo:https://github.com/seanmonstar/reqwest:reqwest; crate::blocking::get; remote; ReturnValue.Field[crate::result::Result::Ok(0)] | -| 2 | Summary: lang:alloc; ::as_str; Argument[self]; ReturnValue; taint | -| 3 | Summary: lang:alloc; crate::fmt::format; Argument[0]; ReturnValue; taint | -| 4 | Summary: lang:core; ::unwrap; Argument[self].Field[crate::result::Result::Ok(0)]; ReturnValue; value | -| 5 | Summary: lang:core; ::unwrap_or; Argument[self].Field[crate::result::Result::Ok(0)]; ReturnValue; value | -| 6 | Summary: lang:core; ::parse; Argument[self]; ReturnValue.Field[crate::result::Result::Ok(0)]; taint | -| 7 | Summary: lang:core; crate::hint::must_use; Argument[0]; ReturnValue; value | -| 8 | Summary: repo:https://github.com/seanmonstar/reqwest:reqwest; ::text; Argument[self]; ReturnValue.Field[crate::result::Result::Ok(0)]; taint | +| 1 | Source: lang:std; crate::env::args; command-line-source; ReturnValue.Element | +| 2 | Source: repo:https://github.com/seanmonstar/reqwest:reqwest; crate::blocking::get; remote; ReturnValue.Field[crate::result::Result::Ok(0)] | +| 3 | Summary: lang:alloc; ::as_str; Argument[self]; ReturnValue; taint | +| 4 | Summary: lang:alloc; crate::fmt::format; Argument[0]; ReturnValue; taint | +| 5 | Summary: lang:core; ::unwrap_or; Argument[self].Field[crate::option::Option::Some(0)]; ReturnValue; value | +| 6 | Summary: lang:core; ::unwrap; Argument[self].Field[crate::result::Result::Ok(0)]; ReturnValue; value | +| 7 | Summary: lang:core; ::unwrap_or; Argument[self].Field[crate::result::Result::Ok(0)]; ReturnValue; value | +| 8 | Summary: lang:core; ::parse; Argument[self]; ReturnValue.Field[crate::result::Result::Ok(0)]; taint | +| 9 | Summary: lang:core; crate::hint::must_use; Argument[0]; ReturnValue; value | +| 10 | Summary: lang:core; crate::iter::traits::iterator::Iterator::nth; Argument[self].Element; ReturnValue.Field[crate::option::Option::Some(0)]; value | +| 11 | Summary: repo:https://github.com/seanmonstar/reqwest:reqwest; ::text; Argument[self]; ReturnValue.Field[crate::result::Result::Ok(0)]; taint | nodes +| sqlx.rs:47:9:47:18 | arg_string | semmle.label | arg_string | +| sqlx.rs:47:22:47:35 | ...::args | semmle.label | ...::args | +| sqlx.rs:47:22:47:37 | ...::args(...) [element] | semmle.label | ...::args(...) [element] | +| sqlx.rs:47:22:47:44 | ... .nth(...) [Some] | semmle.label | ... .nth(...) [Some] | +| sqlx.rs:47:22:47:77 | ... .unwrap_or(...) | semmle.label | ... .unwrap_or(...) | | sqlx.rs:48:9:48:21 | remote_string | semmle.label | remote_string | | sqlx.rs:48:25:48:46 | ...::get | semmle.label | ...::get | | sqlx.rs:48:25:48:69 | ...::get(...) [Ok] | semmle.label | ...::get(...) [Ok] | @@ -62,6 +83,9 @@ nodes | sqlx.rs:52:32:52:87 | ...::must_use(...) | semmle.label | ...::must_use(...) | | sqlx.rs:52:32:52:87 | MacroExpr | semmle.label | MacroExpr | | sqlx.rs:52:32:52:87 | { ... } | semmle.label | { ... } | +| sqlx.rs:53:9:53:22 | unsafe_query_1 [&ref] | semmle.label | unsafe_query_1 [&ref] | +| sqlx.rs:53:26:53:36 | &arg_string [&ref] | semmle.label | &arg_string [&ref] | +| sqlx.rs:53:27:53:36 | arg_string | semmle.label | arg_string | | sqlx.rs:54:9:54:22 | unsafe_query_2 [&ref] | semmle.label | unsafe_query_2 [&ref] | | sqlx.rs:54:26:54:39 | &remote_string [&ref] | semmle.label | &remote_string [&ref] | | sqlx.rs:54:27:54:39 | remote_string | semmle.label | remote_string | @@ -72,10 +96,14 @@ nodes | sqlx.rs:56:34:56:89 | MacroExpr | semmle.label | MacroExpr | | sqlx.rs:56:34:56:89 | { ... } | semmle.label | { ... } | | sqlx.rs:62:26:62:46 | safe_query_3.as_str(...) | semmle.label | safe_query_3.as_str(...) | +| sqlx.rs:63:26:63:39 | unsafe_query_1 [&ref] | semmle.label | unsafe_query_1 [&ref] | +| sqlx.rs:63:26:63:48 | unsafe_query_1.as_str(...) | semmle.label | unsafe_query_1.as_str(...) | | sqlx.rs:65:30:65:43 | unsafe_query_2 [&ref] | semmle.label | unsafe_query_2 [&ref] | | sqlx.rs:65:30:65:52 | unsafe_query_2.as_str(...) | semmle.label | unsafe_query_2.as_str(...) | | sqlx.rs:67:30:67:52 | unsafe_query_4.as_str(...) | semmle.label | unsafe_query_4.as_str(...) | | sqlx.rs:73:25:73:45 | safe_query_3.as_str(...) | semmle.label | safe_query_3.as_str(...) | +| sqlx.rs:74:25:74:38 | unsafe_query_1 [&ref] | semmle.label | unsafe_query_1 [&ref] | +| sqlx.rs:74:25:74:47 | unsafe_query_1.as_str(...) | semmle.label | unsafe_query_1.as_str(...) | | sqlx.rs:76:29:76:42 | unsafe_query_2 [&ref] | semmle.label | unsafe_query_2 [&ref] | | sqlx.rs:76:29:76:51 | unsafe_query_2.as_str(...) | semmle.label | unsafe_query_2.as_str(...) | | sqlx.rs:78:29:78:51 | unsafe_query_4.as_str(...) | semmle.label | unsafe_query_4.as_str(...) | diff --git a/rust/ql/test/query-tests/security/CWE-089/sqlx.rs b/rust/ql/test/query-tests/security/CWE-089/sqlx.rs index d40a4bb2a118..f051cf49e08d 100644 --- a/rust/ql/test/query-tests/security/CWE-089/sqlx.rs +++ b/rust/ql/test/query-tests/security/CWE-089/sqlx.rs @@ -44,7 +44,7 @@ async fn test_sqlx_mysql(url: &str, enable_remote: bool) -> Result<(), sqlx::Err // construct queries (with extra variants) let const_string = String::from("Alice"); - let arg_string = std::env::args().nth(1).unwrap_or(String::from("Alice")); // $ MISSING: Source=args1 + let arg_string = std::env::args().nth(1).unwrap_or(String::from("Alice")); // $ Source=args1 let remote_string = reqwest::blocking::get("http://example.com/").unwrap().text().unwrap_or(String::from("Alice")); // $ Source=remote1 let remote_number = remote_string.parse::().unwrap_or(0); let safe_query_1 = String::from("SELECT * FROM people WHERE firstname='Alice'"); @@ -60,7 +60,7 @@ async fn test_sqlx_mysql(url: &str, enable_remote: bool) -> Result<(), sqlx::Err let _ = conn.execute(safe_query_1.as_str()).await?; // $ sql-sink let _ = conn.execute(safe_query_2.as_str()).await?; // $ sql-sink let _ = conn.execute(safe_query_3.as_str()).await?; // $ sql-sink SPURIOUS: Alert[rust/sql-injection]=remote1 - let _ = conn.execute(unsafe_query_1.as_str()).await?; // $ sql-sink MISSING: Alert[rust/sql-injection]=args1 + let _ = conn.execute(unsafe_query_1.as_str()).await?; // $ sql-sink Alert[rust/sql-injection]=args1 if enable_remote { let _ = conn.execute(unsafe_query_2.as_str()).await?; // $ sql-sink Alert[rust/sql-injection]=remote1 let _ = conn.execute(unsafe_query_3.as_str()).await?; // $ sql-sink MISSING: Alert[rust/sql-injection]=remote1 @@ -71,7 +71,7 @@ async fn test_sqlx_mysql(url: &str, enable_remote: bool) -> Result<(), sqlx::Err let _ = sqlx::query(safe_query_1.as_str()).execute(&pool).await?; // $ sql-sink let _ = sqlx::query(safe_query_2.as_str()).execute(&pool).await?; // $ sql-sink let _ = sqlx::query(safe_query_3.as_str()).execute(&pool).await?; // $ sql-sink SPURIOUS: Alert[rust/sql-injection]=remote1 - let _ = sqlx::query(unsafe_query_1.as_str()).execute(&pool).await?; // $ sql-sink MISSING: Alert[rust/sql-injection][rust/sql-injection]=args1 + let _ = sqlx::query(unsafe_query_1.as_str()).execute(&pool).await?; // $ sql-sink Alert[rust/sql-injection]=args1 if enable_remote { let _ = sqlx::query(unsafe_query_2.as_str()).execute(&pool).await?; // $ sql-sink Alert[rust/sql-injection]=remote1 let _ = sqlx::query(unsafe_query_3.as_str()).execute(&pool).await?; // $ sql-sink MISSING: Alert[rust/sql-injection]=remote1 From 5c83644360fe95a580dd7ef49293bfdafd1caaa7 Mon Sep 17 00:00:00 2001 From: Simon Friis Vindum Date: Mon, 10 Mar 2025 14:52:25 +0100 Subject: [PATCH 5/7] Rust: Use CWE 20 for regex injection query --- .../security/{CWE-730 => CWE-020}/RegexInjection.qhelp | 0 .../queries/security/{CWE-730 => CWE-020}/RegexInjection.ql | 6 +++--- .../security/{CWE-730 => CWE-020}/RegexInjectionBad.rs | 0 .../security/{CWE-730 => CWE-020}/RegexInjectionGood.rs | 0 .../security/{CWE-730 => CWE-020}/RegexInjection.expected | 0 .../security/{CWE-730 => CWE-020}/RegexInjection.qlref | 2 +- .../{CWE-730 => CWE-020}/RegexInjectionSink.expected | 0 .../security/{CWE-730 => CWE-020}/RegexInjectionSink.ql | 0 .../test/query-tests/security/{CWE-730 => CWE-020}/main.rs | 0 .../query-tests/security/{CWE-730 => CWE-020}/options.yml | 0 10 files changed, 4 insertions(+), 4 deletions(-) rename rust/ql/src/queries/security/{CWE-730 => CWE-020}/RegexInjection.qhelp (100%) rename rust/ql/src/queries/security/{CWE-730 => CWE-020}/RegexInjection.ql (94%) rename rust/ql/src/queries/security/{CWE-730 => CWE-020}/RegexInjectionBad.rs (100%) rename rust/ql/src/queries/security/{CWE-730 => CWE-020}/RegexInjectionGood.rs (100%) rename rust/ql/test/query-tests/security/{CWE-730 => CWE-020}/RegexInjection.expected (100%) rename rust/ql/test/query-tests/security/{CWE-730 => CWE-020}/RegexInjection.qlref (52%) rename rust/ql/test/query-tests/security/{CWE-730 => CWE-020}/RegexInjectionSink.expected (100%) rename rust/ql/test/query-tests/security/{CWE-730 => CWE-020}/RegexInjectionSink.ql (100%) rename rust/ql/test/query-tests/security/{CWE-730 => CWE-020}/main.rs (100%) rename rust/ql/test/query-tests/security/{CWE-730 => CWE-020}/options.yml (100%) diff --git a/rust/ql/src/queries/security/CWE-730/RegexInjection.qhelp b/rust/ql/src/queries/security/CWE-020/RegexInjection.qhelp similarity index 100% rename from rust/ql/src/queries/security/CWE-730/RegexInjection.qhelp rename to rust/ql/src/queries/security/CWE-020/RegexInjection.qhelp diff --git a/rust/ql/src/queries/security/CWE-730/RegexInjection.ql b/rust/ql/src/queries/security/CWE-020/RegexInjection.ql similarity index 94% rename from rust/ql/src/queries/security/CWE-730/RegexInjection.ql rename to rust/ql/src/queries/security/CWE-020/RegexInjection.ql index 0d50d8ead6d8..0f3bd1ed85a2 100644 --- a/rust/ql/src/queries/security/CWE-730/RegexInjection.ql +++ b/rust/ql/src/queries/security/CWE-020/RegexInjection.ql @@ -3,12 +3,12 @@ * @description * @kind path-problem * @problem.severity error - * @security-severity 7.5 + * @security-severity 7.8 * @precision high * @id rust/regex-injection * @tags security - * external/cwe/cwe-730 - * external/cwe/cwe-400 + * external/cwe/cwe-020 + * external/cwe/cwe-074 */ private import rust diff --git a/rust/ql/src/queries/security/CWE-730/RegexInjectionBad.rs b/rust/ql/src/queries/security/CWE-020/RegexInjectionBad.rs similarity index 100% rename from rust/ql/src/queries/security/CWE-730/RegexInjectionBad.rs rename to rust/ql/src/queries/security/CWE-020/RegexInjectionBad.rs diff --git a/rust/ql/src/queries/security/CWE-730/RegexInjectionGood.rs b/rust/ql/src/queries/security/CWE-020/RegexInjectionGood.rs similarity index 100% rename from rust/ql/src/queries/security/CWE-730/RegexInjectionGood.rs rename to rust/ql/src/queries/security/CWE-020/RegexInjectionGood.rs diff --git a/rust/ql/test/query-tests/security/CWE-730/RegexInjection.expected b/rust/ql/test/query-tests/security/CWE-020/RegexInjection.expected similarity index 100% rename from rust/ql/test/query-tests/security/CWE-730/RegexInjection.expected rename to rust/ql/test/query-tests/security/CWE-020/RegexInjection.expected diff --git a/rust/ql/test/query-tests/security/CWE-730/RegexInjection.qlref b/rust/ql/test/query-tests/security/CWE-020/RegexInjection.qlref similarity index 52% rename from rust/ql/test/query-tests/security/CWE-730/RegexInjection.qlref rename to rust/ql/test/query-tests/security/CWE-020/RegexInjection.qlref index d62d882f992d..bc028b7e20d6 100644 --- a/rust/ql/test/query-tests/security/CWE-730/RegexInjection.qlref +++ b/rust/ql/test/query-tests/security/CWE-020/RegexInjection.qlref @@ -1,2 +1,2 @@ -query: queries/security/CWE-730/RegexInjection.ql +query: queries/security/CWE-020/RegexInjection.ql postprocess: utils/test/InlineExpectationsTestQuery.ql diff --git a/rust/ql/test/query-tests/security/CWE-730/RegexInjectionSink.expected b/rust/ql/test/query-tests/security/CWE-020/RegexInjectionSink.expected similarity index 100% rename from rust/ql/test/query-tests/security/CWE-730/RegexInjectionSink.expected rename to rust/ql/test/query-tests/security/CWE-020/RegexInjectionSink.expected diff --git a/rust/ql/test/query-tests/security/CWE-730/RegexInjectionSink.ql b/rust/ql/test/query-tests/security/CWE-020/RegexInjectionSink.ql similarity index 100% rename from rust/ql/test/query-tests/security/CWE-730/RegexInjectionSink.ql rename to rust/ql/test/query-tests/security/CWE-020/RegexInjectionSink.ql diff --git a/rust/ql/test/query-tests/security/CWE-730/main.rs b/rust/ql/test/query-tests/security/CWE-020/main.rs similarity index 100% rename from rust/ql/test/query-tests/security/CWE-730/main.rs rename to rust/ql/test/query-tests/security/CWE-020/main.rs diff --git a/rust/ql/test/query-tests/security/CWE-730/options.yml b/rust/ql/test/query-tests/security/CWE-020/options.yml similarity index 100% rename from rust/ql/test/query-tests/security/CWE-730/options.yml rename to rust/ql/test/query-tests/security/CWE-020/options.yml From b48fd99913261250b0c137d9f5594e602f7f1fb4 Mon Sep 17 00:00:00 2001 From: Simon Friis Vindum Date: Mon, 10 Mar 2025 16:30:52 +0100 Subject: [PATCH 6/7] Rust: Applying suggestions to documentation --- .../queries/security/CWE-020/RegexInjection.qhelp | 14 +++++++------- .../src/queries/security/CWE-020/RegexInjection.ql | 5 ++++- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/rust/ql/src/queries/security/CWE-020/RegexInjection.qhelp b/rust/ql/src/queries/security/CWE-020/RegexInjection.qhelp index 046b172d3e7a..f314de2042e2 100644 --- a/rust/ql/src/queries/security/CWE-020/RegexInjection.qhelp +++ b/rust/ql/src/queries/security/CWE-020/RegexInjection.qhelp @@ -6,8 +6,8 @@

    Constructing a regular expression with unsanitized user input can be dangerous. -A malicious user may be able to modify the meaning of the expression causing it -to match unexpected strings and to construct large regular expressions by using +A malicious user may be able to modify the meaning of the expression, causing it +to match unexpected strings and construct large regular expressions by using counted repetitions.

    @@ -22,13 +22,13 @@ escape meta-characters that have special meaning.

    If purposefully supporting user supplied regular expressions, then use RegexBuilder::size_limit -to limit the pattern size such that it is no larger than necessary. +to limit the pattern size so that it is no larger than necessary.

    -The following example construct a regular expressions from the user input +The following example constructs a regular expressions from the user input key without escaping it first.

    @@ -41,8 +41,8 @@ malicious user might inject the regular expression ".*^|key" and unexpectedly cause strings such as "key=secret" to match.

    -If user input is used to construct a regular expression it should be escaped -first. This ensures that the user cannot insert characters that have special +If user input is used to construct a regular expression, it should be escaped +first. This ensures that the malicious users cannot insert characters that have special meanings in regular expressions.

    @@ -50,7 +50,7 @@ meanings in regular expressions.
  • - regex crate documentation: Untrusted patterns + regex crate documentation: Untrusted patterns.
    • diff --git a/rust/ql/src/queries/security/CWE-020/RegexInjection.ql b/rust/ql/src/queries/security/CWE-020/RegexInjection.ql index 0f3bd1ed85a2..b49d08969f62 100644 --- a/rust/ql/src/queries/security/CWE-020/RegexInjection.ql +++ b/rust/ql/src/queries/security/CWE-020/RegexInjection.ql @@ -1,6 +1,9 @@ /** * @name Regular expression injection - * @description + * @description User input should not be used in regular expressions without first being + * escaped, otherwise a malicious user may be able to inject an expression that + * could modify the meaning of the expression, causing it to match unexpected + * strings. * @kind path-problem * @problem.severity error * @security-severity 7.8 From 1e0b78ebd37a206243734bb57a20697b99af2958 Mon Sep 17 00:00:00 2001 From: Simon Friis Vindum Date: Tue, 11 Mar 2025 12:47:12 +0100 Subject: [PATCH 7/7] Rust: Update regex injection description Co-authored-by: Geoffrey White <40627776+geoffw0@users.noreply.github.com> --- rust/ql/src/queries/security/CWE-020/RegexInjection.qhelp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust/ql/src/queries/security/CWE-020/RegexInjection.qhelp b/rust/ql/src/queries/security/CWE-020/RegexInjection.qhelp index f314de2042e2..eb69a607954b 100644 --- a/rust/ql/src/queries/security/CWE-020/RegexInjection.qhelp +++ b/rust/ql/src/queries/security/CWE-020/RegexInjection.qhelp @@ -42,7 +42,7 @@ unexpectedly cause strings such as "key=secret" to match.

    If user input is used to construct a regular expression, it should be escaped -first. This ensures that the malicious users cannot insert characters that have special +first. This ensures that malicious users cannot insert characters that have special meanings in regular expressions.