From dc7d7f121e9d9011269ae73518a7bb77fa08dc5f Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Tue, 11 Mar 2025 11:02:00 +0000 Subject: [PATCH 1/8] Rust: Clarify doc on FlowSink, FlowSource. --- rust/ql/lib/codeql/rust/dataflow/FlowSink.qll | 20 ++++++++++++++-- .../lib/codeql/rust/dataflow/FlowSource.qll | 24 ++++++++++++++++++- 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/rust/ql/lib/codeql/rust/dataflow/FlowSink.qll b/rust/ql/lib/codeql/rust/dataflow/FlowSink.qll index 2a6351930161..0ebe8ffb9289 100644 --- a/rust/ql/lib/codeql/rust/dataflow/FlowSink.qll +++ b/rust/ql/lib/codeql/rust/dataflow/FlowSink.qll @@ -1,4 +1,20 @@ -/** Provides classes and predicates for defining flow sinks. */ +/** + * Provides classes and predicates for defining flow sinks. + * + * Flow sinks defined here feed into data flow configurations as follows: + * + * data from `*.model.yml` | QL extensions of `FlowSink::Range` + * v v + * `FlowSink` (associated with a models-as-data `kind` string) + * v + * `sinkNode` predicate | other QL defined sinks, for example using concepts + * v v + * various `Sink` classes for specific data flow configurations + * + * New sinks should be defined using models-as-data, QL extensions of + * `FlowSink::Range`, or concepts. Data flow configurations should use the + * `sinkNode` predicate and/or concepts to define their sinks. + */ private import rust private import internal.FlowSummaryImpl as Impl @@ -12,7 +28,7 @@ private module Sinks { /** Provides the `Range` class used to define the extent of `FlowSink`. */ module FlowSink { - /** A flow source. */ + /** A flow sink. */ abstract class Range extends Impl::Public::SinkElement { bindingset[this] Range() { any() } diff --git a/rust/ql/lib/codeql/rust/dataflow/FlowSource.qll b/rust/ql/lib/codeql/rust/dataflow/FlowSource.qll index ee356a2a06a6..33262441c9cc 100644 --- a/rust/ql/lib/codeql/rust/dataflow/FlowSource.qll +++ b/rust/ql/lib/codeql/rust/dataflow/FlowSource.qll @@ -1,4 +1,26 @@ -/** Provides classes and predicates for defining flow sources. */ +/** + * Provides classes and predicates for defining flow sources. + * + * Flow sources defined here feed into the `ActiveThreatModelSource` class and + * ultimately reach data flow configurations as follows: + * + * data from `*.model.yml` | QL extensions of `FlowSource::Range` + * v v + * `FlowSource` (associated with a models-as-data `kind` string) + * v + * `sourceNode` predicate | (theoretically other QL defined sources) + * v v + * `ThreatModelSource` (associated with a threat model source type) + * v + * `ActiveThreatModelSource` (just the enabled sources) + * v + * various `Source` classes for specific data flow configurations + * + * New sources should be defined using models-as-data or QL extensions of + * `FlowSource::Range`. Data flow configurations on the other hand should use + * `ActiveThreatModelSource` to match sources enabled in the user + * configuration. + */ private import rust private import internal.FlowSummaryImpl as Impl From 4924a0faf3fc50623c7ca267e5561c58c9914da4 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Tue, 11 Mar 2025 11:39:55 +0000 Subject: [PATCH 2/8] Rust: Introduce a QuerySink class, common to all query sinks. --- rust/ql/lib/codeql/rust/Concepts.qll | 23 +++++++++++++++++++ rust/ql/lib/codeql/rust/dataflow/FlowSink.qll | 2 +- .../security/CleartextLoggingExtensions.qll | 5 +++- .../rust/security/SqlInjectionExtensions.qll | 4 +++- .../ql/src/queries/summary/QuerySinkCounts.ql | 3 ++- rust/ql/src/queries/summary/QuerySinks.ql | 5 ++-- rust/ql/src/queries/summary/Stats.qll | 13 +++-------- 7 files changed, 39 insertions(+), 16 deletions(-) diff --git a/rust/ql/lib/codeql/rust/Concepts.qll b/rust/ql/lib/codeql/rust/Concepts.qll index 1ad206477aee..4f25840165e7 100644 --- a/rust/ql/lib/codeql/rust/Concepts.qll +++ b/rust/ql/lib/codeql/rust/Concepts.qll @@ -152,6 +152,29 @@ class ModeledRemoteSource extends RemoteSource::Range { ModeledRemoteSource() { sourceNode(this, "remote") } } +/** + * A data flow sink that is used in a query. + * + * Extend this class to refine existing API models. If you want to model new APIs, + * extend `QuerySink::Range` instead. + */ +final class QuerySink = QuerySink::Range; + +/** + * Provides a class for modeling new query sinks. + */ +module QuerySink { + /** + * A data flow sink that is used in a query. + */ + abstract class Range extends DataFlow::Node { + /** + * Gets a string that describes the type of this sink (usually the query it applies to). + */ + abstract string getSinkType(); + } +} + /** * A data flow node that constructs a SQL statement (for later execution). * diff --git a/rust/ql/lib/codeql/rust/dataflow/FlowSink.qll b/rust/ql/lib/codeql/rust/dataflow/FlowSink.qll index 0ebe8ffb9289..94e46efcdca7 100644 --- a/rust/ql/lib/codeql/rust/dataflow/FlowSink.qll +++ b/rust/ql/lib/codeql/rust/dataflow/FlowSink.qll @@ -9,7 +9,7 @@ * v * `sinkNode` predicate | other QL defined sinks, for example using concepts * v v - * various `Sink` classes for specific data flow configurations + * various `Sink` classes for specific data flow configurations <- extending `QuerySink` * * New sinks should be defined using models-as-data, QL extensions of * `FlowSink::Range`, or concepts. Data flow configurations should use the diff --git a/rust/ql/lib/codeql/rust/security/CleartextLoggingExtensions.qll b/rust/ql/lib/codeql/rust/security/CleartextLoggingExtensions.qll index bfe6da7ac82c..e6bbc0d2a2be 100644 --- a/rust/ql/lib/codeql/rust/security/CleartextLoggingExtensions.qll +++ b/rust/ql/lib/codeql/rust/security/CleartextLoggingExtensions.qll @@ -7,6 +7,7 @@ import rust private import codeql.rust.dataflow.DataFlow private import codeql.rust.dataflow.internal.DataFlowImpl private import codeql.rust.security.SensitiveData +private import codeql.rust.Concepts /** * Provides default sources, sinks and barriers for detecting cleartext logging @@ -21,7 +22,9 @@ module CleartextLogging { /** * A data flow sink for cleartext logging vulnerabilities. */ - abstract class Sink extends DataFlow::Node { } + abstract class Sink extends QuerySink::Range { + override string getSinkType() { result = "CleartextLogging" } + } /** * A barrier for cleartext logging vulnerabilities. diff --git a/rust/ql/lib/codeql/rust/security/SqlInjectionExtensions.qll b/rust/ql/lib/codeql/rust/security/SqlInjectionExtensions.qll index 4de712080044..9c61fe4aa52d 100644 --- a/rust/ql/lib/codeql/rust/security/SqlInjectionExtensions.qll +++ b/rust/ql/lib/codeql/rust/security/SqlInjectionExtensions.qll @@ -23,7 +23,9 @@ module SqlInjection { /** * A data flow sink for SQL injection vulnerabilities. */ - abstract class Sink extends DataFlow::Node { } + abstract class Sink extends QuerySink::Range { + override string getSinkType() { result = "SqlInjection" } + } /** * A barrier for SQL injection vulnerabilities. diff --git a/rust/ql/src/queries/summary/QuerySinkCounts.ql b/rust/ql/src/queries/summary/QuerySinkCounts.ql index 6525c3263a11..ed1c6bee3900 100644 --- a/rust/ql/src/queries/summary/QuerySinkCounts.ql +++ b/rust/ql/src/queries/summary/QuerySinkCounts.ql @@ -10,8 +10,9 @@ import rust import codeql.rust.dataflow.DataFlow +import codeql.rust.Concepts import Stats from string kind, int num -where num = strictcount(DataFlow::Node n | getAQuerySinkKind(n) = kind) +where num = strictcount(QuerySink s | s.getSinkType() = kind) select kind, num diff --git a/rust/ql/src/queries/summary/QuerySinks.ql b/rust/ql/src/queries/summary/QuerySinks.ql index 09cd7fcb2991..a94ab2f8e804 100644 --- a/rust/ql/src/queries/summary/QuerySinks.ql +++ b/rust/ql/src/queries/summary/QuerySinks.ql @@ -11,7 +11,8 @@ import rust import codeql.rust.dataflow.DataFlow +import codeql.rust.Concepts import Stats -from DataFlow::Node n -select n, "Sink for " + strictconcat(getAQuerySinkKind(n), ", ") + "." +from QuerySink s +select s, "Sink for " + concat(s.getSinkType(), ", ") + "." diff --git a/rust/ql/src/queries/summary/Stats.qll b/rust/ql/src/queries/summary/Stats.qll index f2998a0e4db6..51d99dc4406d 100644 --- a/rust/ql/src/queries/summary/Stats.qll +++ b/rust/ql/src/queries/summary/Stats.qll @@ -9,6 +9,8 @@ private import codeql.rust.dataflow.internal.TaintTrackingImpl private import codeql.rust.internal.AstConsistency as AstConsistency private import codeql.rust.controlflow.internal.CfgConsistency as CfgConsistency private import codeql.rust.dataflow.internal.DataFlowConsistency as DataFlowConsistency +private import codeql.rust.Concepts +// import all query extensions files, so that all extensions of `QuerySink` are found private import codeql.rust.security.SqlInjectionExtensions private import codeql.rust.security.CleartextLoggingExtensions @@ -55,16 +57,7 @@ int getTaintEdgesCount() { ) } -/** - * Gets a kind of query for which `n` is a sink (if any). - */ -string getAQuerySinkKind(DataFlow::Node n) { - n instanceof SqlInjection::Sink and result = "SqlInjection" - or - n instanceof CleartextLogging::Sink and result = "CleartextLogging" -} - /** * Gets a count of the total number of query sinks in the database. */ -int getQuerySinksCount() { result = count(DataFlow::Node n | exists(getAQuerySinkKind(n))) } +int getQuerySinksCount() { result = count(QuerySink s) } From 044d0a13f0fdd5e7690d727cd21121849bd5d9db Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Tue, 11 Mar 2025 11:49:24 +0000 Subject: [PATCH 3/8] Rust: Include WeakSensitiveDataHashing sinks as well. --- .../rust/security/WeakSensitiveDataHashingExtensions.qll | 8 ++++++-- rust/ql/src/queries/summary/Stats.qll | 3 ++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/rust/ql/lib/codeql/rust/security/WeakSensitiveDataHashingExtensions.qll b/rust/ql/lib/codeql/rust/security/WeakSensitiveDataHashingExtensions.qll index 45aa03b58054..d49e932fa4b1 100644 --- a/rust/ql/lib/codeql/rust/security/WeakSensitiveDataHashingExtensions.qll +++ b/rust/ql/lib/codeql/rust/security/WeakSensitiveDataHashingExtensions.qll @@ -43,7 +43,7 @@ module NormalHashFunction { * data" vulnerabilities that applies to data that does not require computationally expensive * hashing. That is, a broken or weak hashing algorithm. */ - abstract class Sink extends DataFlow::Node { + abstract class Sink extends QuerySink::Range { /** * Gets the name of the weak hashing algorithm. */ @@ -76,6 +76,8 @@ module NormalHashFunction { class WeakHashingOperationInputAsSink extends Sink { Cryptography::HashingAlgorithm algorithm; + override string getSinkType() { result = "WeakSensitiveDataHashing" } + WeakHashingOperationInputAsSink() { exists(Cryptography::CryptographicOperation operation | algorithm.isWeak() and @@ -114,7 +116,9 @@ module ComputationallyExpensiveHashFunction { * hashing. That is, a broken or weak hashing algorithm or one that is not computationally * expensive enough for password hashing. */ - abstract class Sink extends DataFlow::Node { + abstract class Sink extends QuerySink::Range { + override string getSinkType() { result = "WeakSensitiveDataHashing" } + /** * Gets the name of the weak hashing algorithm. */ diff --git a/rust/ql/src/queries/summary/Stats.qll b/rust/ql/src/queries/summary/Stats.qll index 51d99dc4406d..668437aac718 100644 --- a/rust/ql/src/queries/summary/Stats.qll +++ b/rust/ql/src/queries/summary/Stats.qll @@ -11,8 +11,9 @@ private import codeql.rust.controlflow.internal.CfgConsistency as CfgConsistency private import codeql.rust.dataflow.internal.DataFlowConsistency as DataFlowConsistency private import codeql.rust.Concepts // import all query extensions files, so that all extensions of `QuerySink` are found -private import codeql.rust.security.SqlInjectionExtensions private import codeql.rust.security.CleartextLoggingExtensions +private import codeql.rust.security.SqlInjectionExtensions +private import codeql.rust.security.WeakSensitiveDataHashingExtensions /** * Gets a count of the total number of lines of code in the database. From df4f117a7c1cb11253e6c0c967d52cd6b1ce13d4 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Wed, 12 Mar 2025 15:52:10 +0000 Subject: [PATCH 4/8] Rust: QLDoc formatting. --- rust/ql/lib/codeql/rust/dataflow/FlowSink.qll | 16 +++++++----- .../lib/codeql/rust/dataflow/FlowSource.qll | 26 ++++++++++--------- 2 files changed, 23 insertions(+), 19 deletions(-) diff --git a/rust/ql/lib/codeql/rust/dataflow/FlowSink.qll b/rust/ql/lib/codeql/rust/dataflow/FlowSink.qll index 94e46efcdca7..d4883b7b1b9c 100644 --- a/rust/ql/lib/codeql/rust/dataflow/FlowSink.qll +++ b/rust/ql/lib/codeql/rust/dataflow/FlowSink.qll @@ -3,13 +3,15 @@ * * Flow sinks defined here feed into data flow configurations as follows: * - * data from `*.model.yml` | QL extensions of `FlowSink::Range` - * v v - * `FlowSink` (associated with a models-as-data `kind` string) - * v - * `sinkNode` predicate | other QL defined sinks, for example using concepts - * v v - * various `Sink` classes for specific data flow configurations <- extending `QuerySink` + * ```text + * data from *.model.yml | QL extensions of FlowSink::Range + * v v + * FlowSink (associated with a models-as-data kind string) + * v + * sinkNode predicate | other QL defined sinks, for example using concepts + * v v + * various Sink classes for specific data flow configurations <- extending QuerySink + * ``` * * New sinks should be defined using models-as-data, QL extensions of * `FlowSink::Range`, or concepts. Data flow configurations should use the diff --git a/rust/ql/lib/codeql/rust/dataflow/FlowSource.qll b/rust/ql/lib/codeql/rust/dataflow/FlowSource.qll index 33262441c9cc..93a5fe5625eb 100644 --- a/rust/ql/lib/codeql/rust/dataflow/FlowSource.qll +++ b/rust/ql/lib/codeql/rust/dataflow/FlowSource.qll @@ -4,18 +4,20 @@ * Flow sources defined here feed into the `ActiveThreatModelSource` class and * ultimately reach data flow configurations as follows: * - * data from `*.model.yml` | QL extensions of `FlowSource::Range` - * v v - * `FlowSource` (associated with a models-as-data `kind` string) - * v - * `sourceNode` predicate | (theoretically other QL defined sources) - * v v - * `ThreatModelSource` (associated with a threat model source type) - * v - * `ActiveThreatModelSource` (just the enabled sources) - * v - * various `Source` classes for specific data flow configurations - * + * ```text + * data from *.model.yml | QL extensions of FlowSource::Range + * v v + * FlowSource (associated with a models-as-data kind string) + * v + * sourceNode predicate | (theoretically other QL defined sources) + * v v + * ThreatModelSource (associated with a threat model source type) + * v + * ActiveThreatModelSource (just the enabled sources) + * v + * various Source classes for specific data flow configurations + * ``` + * New sources should be defined using models-as-data or QL extensions of * `FlowSource::Range`. Data flow configurations on the other hand should use * `ActiveThreatModelSource` to match sources enabled in the user From 56f6a67d5f7a3918d607e2dc2e7e0052260aa874 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Wed, 12 Mar 2025 16:08:33 +0000 Subject: [PATCH 5/8] Rust: Add sinks for rust/regex-injection to stats. --- rust/ql/src/queries/summary/Stats.qll | 1 + 1 file changed, 1 insertion(+) diff --git a/rust/ql/src/queries/summary/Stats.qll b/rust/ql/src/queries/summary/Stats.qll index 668437aac718..e129490247ac 100644 --- a/rust/ql/src/queries/summary/Stats.qll +++ b/rust/ql/src/queries/summary/Stats.qll @@ -14,6 +14,7 @@ private import codeql.rust.Concepts private import codeql.rust.security.CleartextLoggingExtensions private import codeql.rust.security.SqlInjectionExtensions private import codeql.rust.security.WeakSensitiveDataHashingExtensions +private import codeql.rust.security.regex.RegexInjectionExtensions /** * Gets a count of the total number of lines of code in the database. From ee6455a7b107b1dd2a6cf4198850cfc07b5247f6 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Wed, 12 Mar 2025 16:23:41 +0000 Subject: [PATCH 6/8] Rust: ... and extend QuerySink to complete the above. --- .../codeql/rust/security/regex/RegexInjectionExtensions.qll | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/rust/ql/lib/codeql/rust/security/regex/RegexInjectionExtensions.qll b/rust/ql/lib/codeql/rust/security/regex/RegexInjectionExtensions.qll index 7e886146c2cf..e0180f88e1f4 100644 --- a/rust/ql/lib/codeql/rust/security/regex/RegexInjectionExtensions.qll +++ b/rust/ql/lib/codeql/rust/security/regex/RegexInjectionExtensions.qll @@ -8,11 +8,12 @@ private import rust private import codeql.rust.dataflow.DataFlow private import codeql.rust.controlflow.CfgNodes private import codeql.rust.dataflow.FlowSink +private import codeql.rust.Concepts /** * A data flow sink for regular expression injection vulnerabilities. */ -abstract class RegexInjectionSink extends DataFlow::Node { } +abstract class RegexInjectionSink extends QuerySink { } /** * A barrier for regular expression injection vulnerabilities. From 64b57679bf0c391e29347a71aba19cfc9de0559d Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Wed, 12 Mar 2025 16:32:53 +0000 Subject: [PATCH 7/8] Rust: ... one more fix. --- .../codeql/rust/security/regex/RegexInjectionExtensions.qll | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/rust/ql/lib/codeql/rust/security/regex/RegexInjectionExtensions.qll b/rust/ql/lib/codeql/rust/security/regex/RegexInjectionExtensions.qll index e0180f88e1f4..8b598fcbeae7 100644 --- a/rust/ql/lib/codeql/rust/security/regex/RegexInjectionExtensions.qll +++ b/rust/ql/lib/codeql/rust/security/regex/RegexInjectionExtensions.qll @@ -13,7 +13,9 @@ private import codeql.rust.Concepts /** * A data flow sink for regular expression injection vulnerabilities. */ -abstract class RegexInjectionSink extends QuerySink { } +abstract class RegexInjectionSink extends QuerySink::Range { + override string getSinkType() { result = "RegexInjection" } + } /** * A barrier for regular expression injection vulnerabilities. From 0df652b2979bf1afa6aa75d26c94e9eae116f4a1 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Wed, 12 Mar 2025 16:38:00 +0000 Subject: [PATCH 8/8] Rust: Autoformat. --- rust/ql/lib/codeql/rust/dataflow/FlowSource.qll | 2 +- .../lib/codeql/rust/security/regex/RegexInjectionExtensions.qll | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/rust/ql/lib/codeql/rust/dataflow/FlowSource.qll b/rust/ql/lib/codeql/rust/dataflow/FlowSource.qll index 93a5fe5625eb..51562fdf83a3 100644 --- a/rust/ql/lib/codeql/rust/dataflow/FlowSource.qll +++ b/rust/ql/lib/codeql/rust/dataflow/FlowSource.qll @@ -17,7 +17,7 @@ * v * various Source classes for specific data flow configurations * ``` - + * * New sources should be defined using models-as-data or QL extensions of * `FlowSource::Range`. Data flow configurations on the other hand should use * `ActiveThreatModelSource` to match sources enabled in the user diff --git a/rust/ql/lib/codeql/rust/security/regex/RegexInjectionExtensions.qll b/rust/ql/lib/codeql/rust/security/regex/RegexInjectionExtensions.qll index 8b598fcbeae7..5ab77b63b6ad 100644 --- a/rust/ql/lib/codeql/rust/security/regex/RegexInjectionExtensions.qll +++ b/rust/ql/lib/codeql/rust/security/regex/RegexInjectionExtensions.qll @@ -15,7 +15,7 @@ private import codeql.rust.Concepts */ abstract class RegexInjectionSink extends QuerySink::Range { override string getSinkType() { result = "RegexInjection" } - } +} /** * A barrier for regular expression injection vulnerabilities.