From 21d8ffa6a635217786d4f8a893d62c20cb89b646 Mon Sep 17 00:00:00 2001 From: GeekMasher Date: Tue, 15 Apr 2025 11:18:05 +0100 Subject: [PATCH 1/6] feat: add utility predicates and classes for Java library --- java/lib/ghsl.qll | 3 + java/lib/ghsl/Utils.qll | 119 ++++++++++++++++++++++++++++++++++++++++ java/lib/qlpack.yml | 1 + 3 files changed, 123 insertions(+) create mode 100644 java/lib/ghsl.qll create mode 100644 java/lib/ghsl/Utils.qll diff --git a/java/lib/ghsl.qll b/java/lib/ghsl.qll new file mode 100644 index 00000000..01fe18fe --- /dev/null +++ b/java/lib/ghsl.qll @@ -0,0 +1,3 @@ +import ghsl.LocalSources +// Export utils +import ghsl.Utils \ No newline at end of file diff --git a/java/lib/ghsl/Utils.qll b/java/lib/ghsl/Utils.qll new file mode 100644 index 00000000..4c82795a --- /dev/null +++ b/java/lib/ghsl/Utils.qll @@ -0,0 +1,119 @@ +/** + * A collection of utility predicates and classes for the Java library. + */ + +private import semmle.code.java.dataflow.DataFlow +private import semmle.code.java.dataflow.ExternalFlow +private import semmle.code.java.dataflow.FlowSources +// Sinks +private import semmle.code.java.security.QueryInjection +private import semmle.code.java.security.CommandLineQuery +private import semmle.code.java.security.LdapInjection +private import semmle.code.java.security.LogInjection +private import semmle.code.java.security.OgnlInjection +private import semmle.code.java.security.RequestForgery +private import semmle.code.java.security.TemplateInjection + +/** + * Filter nodes by its location (relative path or base name). + */ +bindingset[relative_path] +predicate findByLocation(DataFlow::Node node, string relative_path, int linenumber) { + node.getLocation().getFile().getRelativePath().matches(relative_path) and + node.getLocation().getStartLine() = linenumber +} + +/** + * This will only show sinks that are callable (method calls) + */ +predicate isCallable(DataFlow::Node sink) { sink.asExpr() instanceof MethodCall } + +/** + * Check if the source node is a method parameter. + */ +predicate checkSource(DataFlow::Node source) { + // TODO: fix this + source.asParameter() instanceof Parameter + or + source.asExpr() instanceof MethodCall +} + +/** + * Local sources + */ +class LocalSources = LocalUserInput; + +/** + * List of all the souces + */ +class AllSources extends DataFlow::Node { + private string threadmodel; + + AllSources() { + this instanceof LocalUserInput and + threadmodel = "local" + or + this instanceof RemoteFlowSource and + threadmodel = "remote" + or + this instanceof ActiveThreatModelSource + and + threadmodel = this.(SourceNode).getThreatModel() + } + + /** + * Gets the source threat model. + */ + string getThreatModel() { + result = threadmodel + } +} + +/** + * List of all the sinks that we want to check. + */ +class AllSinks extends DataFlow::Node { + private string sink; + + AllSinks() { + this instanceof QueryInjectionSink + and + sink = "QueryInjectionSink" + or + this instanceof CommandInjectionSink + and + sink = "CommandInjectionSink" + or + this instanceof LdapInjectionSink + and + sink = "LdapInjectionSink" + or + this instanceof LogInjectionSink + and + sink = "LogInjectionSink" + or + this instanceof OgnlInjectionSink + and + sink = "OgnlInjectionSink" + or + this instanceof RequestForgerySink + and + sink = "RequestForgerySink" + or + this instanceof TemplateInjectionSink + and + sink = "TemplateInjectionSink" + or + // All MaD sinks + sinkNode(this, _) + and + sink = "MaD" + } + + /** + * Gets the sink sink type. + */ + string sinkType() { + result = sink + } +} diff --git a/java/lib/qlpack.yml b/java/lib/qlpack.yml index 38bd3ea0..5480b5de 100644 --- a/java/lib/qlpack.yml +++ b/java/lib/qlpack.yml @@ -3,3 +3,4 @@ name: githubsecuritylab/codeql-java-libs version: 0.2.1 dependencies: codeql/java-all: '*' + githubsecuritylab/codeql-java-extensions: '0.2.1' From c05a9c2f0d0c7c7a859973b7a0507ea784697847 Mon Sep 17 00:00:00 2001 From: GeekMasher Date: Tue, 15 Apr 2025 11:20:35 +0100 Subject: [PATCH 2/6] feat: add debugging sources and sinks queries for Java --- java/src/debugging/Sinks.ql | 16 ++++++++++++++++ java/src/debugging/Sources.ql | 19 +++++++++++++++++++ java/src/suites/java-debugging.qls | 13 +++++++++++++ 3 files changed, 48 insertions(+) create mode 100644 java/src/debugging/Sinks.ql create mode 100644 java/src/debugging/Sources.ql create mode 100644 java/src/suites/java-debugging.qls diff --git a/java/src/debugging/Sinks.ql b/java/src/debugging/Sinks.ql new file mode 100644 index 00000000..552d483c --- /dev/null +++ b/java/src/debugging/Sinks.ql @@ -0,0 +1,16 @@ +/** + * @name List of all known sinks + * @kind problem + * @problem.severity warning + * @security-severity 1.0 + * @sub-severity low + * @precision low + * @id java/debugging/sinks + * @tags debugging + */ + +import java +import ghsl + +from AllSinks sinks +select sinks, "sink[" + sinks.sinkType() + "]" diff --git a/java/src/debugging/Sources.ql b/java/src/debugging/Sources.ql new file mode 100644 index 00000000..d407185b --- /dev/null +++ b/java/src/debugging/Sources.ql @@ -0,0 +1,19 @@ +/** + * @name List of all known sources (remote, local, etc.) + * @kind problem + * @problem.severity warning + * @security-severity 1.0 + * @sub-severity low + * @precision low + * @id java/debugging/sources + * @tags debugging + */ + +import java +import ghsl + +from AllSources sources, string threatModel +where threatModel = sources.getThreatModel() +// Local sources +// sources.getThreatModel() = "local" +select sources, "source[" + threatModel + "]" diff --git a/java/src/suites/java-debugging.qls b/java/src/suites/java-debugging.qls new file mode 100644 index 00000000..919b0ace --- /dev/null +++ b/java/src/suites/java-debugging.qls @@ -0,0 +1,13 @@ +- description: "GitHub's Community Packs Java/Kotlin Extended Suite" + +- queries: '.' + from: githubsecuritylab/codeql-java-queries + +- include: + tags contain: + - debugging + +# Remove local testing folders +- exclude: + query path: + - /testing\/.*/ From 5cb4ed6061a14f984cfa424d7872e1ff59b0f822 Mon Sep 17 00:00:00 2001 From: GeekMasher Date: Tue, 15 Apr 2025 11:35:42 +0100 Subject: [PATCH 3/6] feat(java): enhance precision levels for debugging sources and sinks in Java --- java/src/debugging/Sinks.ql | 2 +- java/src/debugging/Sources.ql | 2 +- java/src/suites/java-debugging.qls | 6 ++++++ 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/java/src/debugging/Sinks.ql b/java/src/debugging/Sinks.ql index 552d483c..ffd51462 100644 --- a/java/src/debugging/Sinks.ql +++ b/java/src/debugging/Sinks.ql @@ -4,7 +4,7 @@ * @problem.severity warning * @security-severity 1.0 * @sub-severity low - * @precision low + * @precision high * @id java/debugging/sinks * @tags debugging */ diff --git a/java/src/debugging/Sources.ql b/java/src/debugging/Sources.ql index d407185b..b18cf12d 100644 --- a/java/src/debugging/Sources.ql +++ b/java/src/debugging/Sources.ql @@ -4,7 +4,7 @@ * @problem.severity warning * @security-severity 1.0 * @sub-severity low - * @precision low + * @precision high * @id java/debugging/sources * @tags debugging */ diff --git a/java/src/suites/java-debugging.qls b/java/src/suites/java-debugging.qls index 919b0ace..0a48e6be 100644 --- a/java/src/suites/java-debugging.qls +++ b/java/src/suites/java-debugging.qls @@ -4,6 +4,12 @@ from: githubsecuritylab/codeql-java-queries - include: + kind: + - problem + - path-problem + precision: + - very-high + - high tags contain: - debugging From 486a8a763c40318bd87373ad670264871c4188be Mon Sep 17 00:00:00 2001 From: GeekMasher Date: Tue, 15 Apr 2025 11:35:59 +0100 Subject: [PATCH 4/6] feat(java): Add PartialPaths queries --- java/src/debugging/PartialPathsFromSink.ql | 41 +++++++++++++++++ java/src/debugging/PartialPathsFromSource.ql | 48 ++++++++++++++++++++ 2 files changed, 89 insertions(+) create mode 100644 java/src/debugging/PartialPathsFromSink.ql create mode 100644 java/src/debugging/PartialPathsFromSource.ql diff --git a/java/src/debugging/PartialPathsFromSink.ql b/java/src/debugging/PartialPathsFromSink.ql new file mode 100644 index 00000000..6efd534e --- /dev/null +++ b/java/src/debugging/PartialPathsFromSink.ql @@ -0,0 +1,41 @@ +/** + * @name Partial Path Query from Sink + * @kind path-problem + * @problem.severity warning + * @security-severity 1.0 + * @sub-severity low + * @precision low + * @id java/debugging/partial-path-from-sink + * @tags debugging + */ + +import java +import ghsl +import semmle.code.java.dataflow.DataFlow +import semmle.code.java.dataflow.FlowSources +import semmle.code.java.dataflow.TaintTracking + +// Partial Graph +private module RemoteFlowsConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { any() } + + predicate isSink(DataFlow::Node sink) { sink instanceof AllSinks } +} + +int explorationLimit() { result = 10 } + +private module RemoteFlows = DataFlow::Global; + +private module RemoteFlowsPartial = RemoteFlows::FlowExplorationRev; + +private import RemoteFlowsPartial::PartialPathGraph + +from RemoteFlowsPartial::PartialPathNode source, RemoteFlowsPartial::PartialPathNode sink +where + // Only show sinks from a certain file + findByLocation(sink.getNode(), "File.java", _) and + // Only show sources that match our criteria + // checkSource(source.getNode()) and + // Partical Path + RemoteFlowsPartial::partialFlow(source, sink, _) +select sink.getNode(), source, sink, "Partial Graph $@.", source.getNode(), "user-provided value" diff --git a/java/src/debugging/PartialPathsFromSource.ql b/java/src/debugging/PartialPathsFromSource.ql new file mode 100644 index 00000000..af0302f0 --- /dev/null +++ b/java/src/debugging/PartialPathsFromSource.ql @@ -0,0 +1,48 @@ +/** + * @name Partial Path Query from Source + * @kind path-problem + * @problem.severity warning + * @security-severity 1.0 + * @sub-severity low + * @precision low + * @id java/debugging/partial-path-from-source + * @tags debugging + */ + +import java +import ghsl +import semmle.code.java.dataflow.DataFlow +import semmle.code.java.dataflow.FlowSources +import semmle.code.java.dataflow.TaintTracking + +class Sources extends AllSources { + Sources() { + findByLocation(this, "App.java", _) + } + +} + +// Partial Graph +private module RemoteFlowsConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source instanceof Sources } + + predicate isSink(DataFlow::Node sink) { none() } +} + +int explorationLimit() { result = 10 } + +private module RemoteFlows = DataFlow::Global; + +private module RemoteFlowsPartial = RemoteFlows::FlowExplorationFwd; + +private import RemoteFlowsPartial::PartialPathGraph + +from RemoteFlowsPartial::PartialPathNode source, RemoteFlowsPartial::PartialPathNode sink +where + // Filter by file (line number) + // findByLocation(source.getNode(), "File.java", _) and + // Filter by if the sink is callable + // isCallable(sink.getNode()) and + // Perform Partial Flow query + RemoteFlowsPartial::partialFlow(source, sink, _) +select sink.getNode(), source, sink, "Partial Graph $@.", source.getNode(), "user-provided value" From f87db8ea2dbefd30ff33e89177bb3f87f751cfd1 Mon Sep 17 00:00:00 2001 From: GeekMasher Date: Tue, 15 Apr 2025 11:39:38 +0100 Subject: [PATCH 5/6] refactor(java): update comments and improve source filtering in PartialPaths queries --- java/src/debugging/PartialPathsFromSink.ql | 8 ++++---- java/src/debugging/PartialPathsFromSource.ql | 15 ++++----------- 2 files changed, 8 insertions(+), 15 deletions(-) diff --git a/java/src/debugging/PartialPathsFromSink.ql b/java/src/debugging/PartialPathsFromSink.ql index 6efd534e..a3f4499a 100644 --- a/java/src/debugging/PartialPathsFromSink.ql +++ b/java/src/debugging/PartialPathsFromSink.ql @@ -32,10 +32,10 @@ private import RemoteFlowsPartial::PartialPathGraph from RemoteFlowsPartial::PartialPathNode source, RemoteFlowsPartial::PartialPathNode sink where - // Only show sinks from a certain file - findByLocation(sink.getNode(), "File.java", _) and - // Only show sources that match our criteria + /// Only show sinks from a certain file + // findByLocation(sink.getNode(), "File.java", _) and + /// Only show sources that match our criteria // checkSource(source.getNode()) and - // Partical Path + /// Partical Path RemoteFlowsPartial::partialFlow(source, sink, _) select sink.getNode(), source, sink, "Partial Graph $@.", source.getNode(), "user-provided value" diff --git a/java/src/debugging/PartialPathsFromSource.ql b/java/src/debugging/PartialPathsFromSource.ql index af0302f0..6fe9eb50 100644 --- a/java/src/debugging/PartialPathsFromSource.ql +++ b/java/src/debugging/PartialPathsFromSource.ql @@ -15,16 +15,9 @@ import semmle.code.java.dataflow.DataFlow import semmle.code.java.dataflow.FlowSources import semmle.code.java.dataflow.TaintTracking -class Sources extends AllSources { - Sources() { - findByLocation(this, "App.java", _) - } - -} - // Partial Graph private module RemoteFlowsConfig implements DataFlow::ConfigSig { - predicate isSource(DataFlow::Node source) { source instanceof Sources } + predicate isSource(DataFlow::Node source) { source instanceof AllSources } predicate isSink(DataFlow::Node sink) { none() } } @@ -39,10 +32,10 @@ private import RemoteFlowsPartial::PartialPathGraph from RemoteFlowsPartial::PartialPathNode source, RemoteFlowsPartial::PartialPathNode sink where - // Filter by file (line number) + /// Filter by file (line number) // findByLocation(source.getNode(), "File.java", _) and - // Filter by if the sink is callable + /// Filter by if the sink is callable // isCallable(sink.getNode()) and - // Perform Partial Flow query + /// Perform Partial Flow query RemoteFlowsPartial::partialFlow(source, sink, _) select sink.getNode(), source, sink, "Partial Graph $@.", source.getNode(), "user-provided value" From 004fb59da09b938d3ff8848c24f9ac5d90386a7e Mon Sep 17 00:00:00 2001 From: GeekMasher Date: Tue, 15 Apr 2025 11:50:02 +0100 Subject: [PATCH 6/6] fix(java): improve source check logic in Utils.qll --- java/lib/ghsl/Utils.qll | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/java/lib/ghsl/Utils.qll b/java/lib/ghsl/Utils.qll index 4c82795a..184fe4d6 100644 --- a/java/lib/ghsl/Utils.qll +++ b/java/lib/ghsl/Utils.qll @@ -32,8 +32,7 @@ predicate isCallable(DataFlow::Node sink) { sink.asExpr() instanceof MethodCall * Check if the source node is a method parameter. */ predicate checkSource(DataFlow::Node source) { - // TODO: fix this - source.asParameter() instanceof Parameter + exists(source.asParameter()) or source.asExpr() instanceof MethodCall }