From 54b5fd9e7ff90eeb7c46f23d104c7c8a3eaca44e Mon Sep 17 00:00:00 2001 From: Asger F Date: Mon, 16 Dec 2024 14:28:53 +0100 Subject: [PATCH 1/4] DataFlow: Include PathGraph in GlobalFlowSig This aims to remove the need for `::PathGraph` in common cases, so users just have to import the result of Global<...>. --- shared/dataflow/codeql/dataflow/DataFlow.qll | 24 ++++++++++++++++++- .../codeql/dataflow/internal/DataFlowImpl.qll | 2 ++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/shared/dataflow/codeql/dataflow/DataFlow.qll b/shared/dataflow/codeql/dataflow/DataFlow.qll index 7a2f78089778..ca6e53d64fde 100644 --- a/shared/dataflow/codeql/dataflow/DataFlow.qll +++ b/shared/dataflow/codeql/dataflow/DataFlow.qll @@ -615,7 +615,16 @@ module DataFlowMake Lang> { * A `Node` augmented with a call context (except for sinks) and an access path. * Only those `PathNode`s that are reachable from a source, and which can reach a sink, are generated. */ - class PathNode; + class PathNode { + /** Gets a textual representation of this element. */ + string toString(); + + /** Gets the underlying `Node`. */ + Node getNode(); + + /** Gets the location of this node. */ + Location getLocation(); + } /** * Holds if data can flow from `source` to `sink`. @@ -639,6 +648,19 @@ module DataFlowMake Lang> { * Holds if data can flow from some source to `sink`. */ predicate flowToExpr(DataFlowExpr sink); + + /** Holds if `(a,b)` is an edge in the graph of data flow path explanations. */ + predicate edges(PathNode a, PathNode b, string key, string val); + + /** Holds if `n` is a node in the graph of data flow path explanations. */ + predicate nodes(PathNode n, string key, string val); + + /** + * Holds if `(arg, par, ret, out)` forms a subpath-tuple, that is, flow through + * a subpath between `par` and `ret` with the connecting edges `arg -> par` and + * `ret -> out` is summarized as the edge `arg -> out`. + */ + predicate subpaths(PathNode arg, PathNode par, PathNode ret, PathNode out); } /** diff --git a/shared/dataflow/codeql/dataflow/internal/DataFlowImpl.qll b/shared/dataflow/codeql/dataflow/internal/DataFlowImpl.qll index 1373345423f7..c74e8e4fbd79 100644 --- a/shared/dataflow/codeql/dataflow/internal/DataFlowImpl.qll +++ b/shared/dataflow/codeql/dataflow/internal/DataFlowImpl.qll @@ -168,6 +168,8 @@ module MakeImpl Lang> { module Impl { private class FlowState = Config::FlowState; + import PathGraph + private module SourceSinkFiltering { private import codeql.util.AlertFiltering From fd2b8d7f64e835e6e6433671a1d7fe47609afa16 Mon Sep 17 00:00:00 2001 From: Asger F Date: Mon, 16 Dec 2024 15:13:11 +0100 Subject: [PATCH 2/4] MergeFlow --- shared/dataflow/codeql/dataflow/DataFlow.qll | 29 ++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/shared/dataflow/codeql/dataflow/DataFlow.qll b/shared/dataflow/codeql/dataflow/DataFlow.qll index ca6e53d64fde..501aba0acd95 100644 --- a/shared/dataflow/codeql/dataflow/DataFlow.qll +++ b/shared/dataflow/codeql/dataflow/DataFlow.qll @@ -727,6 +727,35 @@ module DataFlowMake Lang> { import PathGraphSigMod + private module GetPathGraph implements PathGraphSig { + import Flow + } + + /** + * Constructs a graph containing the disjoint union of two graphs. + */ + module MergeFlows implements GlobalFlowSig { + private module PathGraph1 = GetPathGraph; + + private module PathGraph2 = GetPathGraph; + + import MergePathGraph + import PathGraph + + predicate flowPath(PathNode source, PathNode sink) { + Graph1::flowPath(source.asPathNode1(), sink.asPathNode1()) or + Graph2::flowPath(source.asPathNode2(), sink.asPathNode2()) + } + + predicate flow(Node source, Node sink) { + Graph1::flow(source, sink) or Graph2::flow(source, sink) + } + + predicate flowTo(Node sink) { Graph1::flowTo(sink) or Graph2::flowTo(sink) } + + predicate flowToExpr(DataFlowExpr sink) { Graph1::flowToExpr(sink) or Graph2::flowToExpr(sink) } + } + /** * Constructs a `PathGraph` from two `PathGraph`s by disjoint union. */ From c417dc4da048980eaec997431ed589b1b3fb1ae8 Mon Sep 17 00:00:00 2001 From: Asger F Date: Mon, 16 Dec 2024 15:13:28 +0100 Subject: [PATCH 3/4] Ruby: use in ReflectedXss --- ruby/ql/src/queries/security/cwe-079/ReflectedXSS.ql | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ruby/ql/src/queries/security/cwe-079/ReflectedXSS.ql b/ruby/ql/src/queries/security/cwe-079/ReflectedXSS.ql index 8cc60618cc5c..d0557679604f 100644 --- a/ruby/ql/src/queries/security/cwe-079/ReflectedXSS.ql +++ b/ruby/ql/src/queries/security/cwe-079/ReflectedXSS.ql @@ -15,9 +15,9 @@ import codeql.ruby.AST import codeql.ruby.security.ReflectedXSSQuery -import ReflectedXssFlow::PathGraph +import ReflectedXssFlow -from ReflectedXssFlow::PathNode source, ReflectedXssFlow::PathNode sink -where ReflectedXssFlow::flowPath(source, sink) +from PathNode source, PathNode sink +where flowPath(source, sink) select sink.getNode(), source, sink, "Cross-site scripting vulnerability due to a $@.", source.getNode(), "user-provided value" From 646395bd11b370322fdcc48e26ff8c22eb5b216a Mon Sep 17 00:00:00 2001 From: Asger F Date: Mon, 16 Dec 2024 15:13:37 +0100 Subject: [PATCH 4/4] Ruby: use in SensitiveDataHashing --- .../codeql/ruby/security/WeakSensitiveDataHashingQuery.qll | 4 +--- .../queries/security/cwe-327/WeakSensitiveDataHashing.ql | 6 ++---- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/ruby/ql/lib/codeql/ruby/security/WeakSensitiveDataHashingQuery.qll b/ruby/ql/lib/codeql/ruby/security/WeakSensitiveDataHashingQuery.qll index dd9c389b4c34..33218b8e8e84 100644 --- a/ruby/ql/lib/codeql/ruby/security/WeakSensitiveDataHashingQuery.qll +++ b/ruby/ql/lib/codeql/ruby/security/WeakSensitiveDataHashingQuery.qll @@ -68,9 +68,7 @@ module ComputationallyExpensiveHashFunction { * `computationallyExpensiveHashFunctionFlowPath`. */ module WeakSensitiveDataHashingFlow = - DataFlow::MergePathGraph; + DataFlow::MergeFlows; /** Holds if data can flow from `source` to `sink` with `NormalHashFunction::Flow`. */ predicate normalHashFunctionFlowPath( diff --git a/ruby/ql/src/queries/security/cwe-327/WeakSensitiveDataHashing.ql b/ruby/ql/src/queries/security/cwe-327/WeakSensitiveDataHashing.ql index 64d9615837df..14a87df80169 100644 --- a/ruby/ql/src/queries/security/cwe-327/WeakSensitiveDataHashing.ql +++ b/ruby/ql/src/queries/security/cwe-327/WeakSensitiveDataHashing.ql @@ -14,11 +14,9 @@ import ruby import codeql.ruby.security.WeakSensitiveDataHashingQuery -import WeakSensitiveDataHashingFlow::PathGraph +import WeakSensitiveDataHashingFlow -from - WeakSensitiveDataHashingFlow::PathNode source, WeakSensitiveDataHashingFlow::PathNode sink, - string ending, string algorithmName, string classification +from PathNode source, PathNode sink, string ending, string algorithmName, string classification where normalHashFunctionFlowPath(source, sink) and algorithmName = sink.getNode().(NormalHashFunction::Sink).getAlgorithmName() and