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/src/queries/security/CWE-020/RegexInjection.qhelp b/rust/ql/src/queries/security/CWE-020/RegexInjection.qhelp new file mode 100644 index 000000000000..eb69a607954b --- /dev/null +++ b/rust/ql/src/queries/security/CWE-020/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 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 so that it is no larger than necessary. +

+
+ + +

+The following example constructs 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 malicious users cannot insert characters that have special +meanings in regular expressions. +

+ +
+ + +
  • + 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 new file mode 100644 index 000000000000..b49d08969f62 --- /dev/null +++ b/rust/ql/src/queries/security/CWE-020/RegexInjection.ql @@ -0,0 +1,48 @@ +/** + * @name Regular expression injection + * @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 + * @precision high + * @id rust/regex-injection + * @tags security + * external/cwe/cwe-020 + * external/cwe/cwe-074 + */ + +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 ActiveThreatModelSource } + + 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 +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-020/RegexInjectionBad.rs b/rust/ql/src/queries/security/CWE-020/RegexInjectionBad.rs new file mode 100644 index 000000000000..5d5bd78becc6 --- /dev/null +++ b/rust/ql/src/queries/security/CWE-020/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-020/RegexInjectionGood.rs b/rust/ql/src/queries/security/CWE-020/RegexInjectionGood.rs new file mode 100644 index 000000000000..fa87435acdc0 --- /dev/null +++ b/rust/ql/src/queries/security/CWE-020/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/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/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/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/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 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-020/RegexInjection.expected b/rust/ql/test/query-tests/security/CWE-020/RegexInjection.expected new file mode 100644 index 000000000000..fd7ded696334 --- /dev/null +++ b/rust/ql/test/query-tests/security/CWE-020/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-020/RegexInjection.qlref b/rust/ql/test/query-tests/security/CWE-020/RegexInjection.qlref new file mode 100644 index 000000000000..bc028b7e20d6 --- /dev/null +++ b/rust/ql/test/query-tests/security/CWE-020/RegexInjection.qlref @@ -0,0 +1,2 @@ +query: queries/security/CWE-020/RegexInjection.ql +postprocess: utils/test/InlineExpectationsTestQuery.ql diff --git a/rust/ql/test/query-tests/security/CWE-020/RegexInjectionSink.expected b/rust/ql/test/query-tests/security/CWE-020/RegexInjectionSink.expected new file mode 100644 index 000000000000..4ea46a385870 --- /dev/null +++ b/rust/ql/test/query-tests/security/CWE-020/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-020/RegexInjectionSink.ql b/rust/ql/test/query-tests/security/CWE-020/RegexInjectionSink.ql new file mode 100644 index 000000000000..0723ec5bb5b7 --- /dev/null +++ b/rust/ql/test/query-tests/security/CWE-020/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-020/main.rs b/rust/ql/test/query-tests/security/CWE-020/main.rs new file mode 100644 index 000000000000..a03e5d4b4942 --- /dev/null +++ b/rust/ql/test/query-tests/security/CWE-020/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-020/options.yml b/rust/ql/test/query-tests/security/CWE-020/options.yml new file mode 100644 index 000000000000..8d484623101f --- /dev/null +++ b/rust/ql/test/query-tests/security/CWE-020/options.yml @@ -0,0 +1,3 @@ +qltest_cargo_check: true +qltest_dependencies: + - regex = { version = "1.11.1" } 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