Java: Convert security queries to path-problem.

change-notes/1.19/analysis-java.md

@@ -2,6 +2,8 @@
 
 ## General improvements
 
+* Where applicable, path explanations have been added to the security queries.
+
 ## New queries
 
 | **Query**                   | **Tags**  | **Purpose**                                                        |

java/ql/src/Security/CWE/CWE-022/TaintedPath.ql

@@ -1,7 +1,7 @@
 /**
  * @name Uncontrolled data used in path expression
  * @description Accessing paths influenced by users can allow an attacker to access unexpected resources.
- * @kind problem
+ * @kind path-problem
  * @problem.severity error
  * @precision high
  * @id java/path-injection
@@ -15,6 +15,7 @@
 import java
 import semmle.code.java.dataflow.FlowSources
 import PathsCommon
+import DataFlow::PathGraph
 
 class TaintedPathConfig extends TaintTracking::Configuration {
   TaintedPathConfig() { this = "TaintedPathConfig" }
@@ -30,8 +31,9 @@ class TaintedPathConfig extends TaintTracking::Configuration {
   }
 }
 
-from RemoteUserInput u, PathCreation p, Expr e, TaintedPathConfig conf
+from DataFlow::PathNode source, DataFlow::PathNode sink, PathCreation p, TaintedPathConfig conf
 where
-  e = p.getInput() and
-  conf.hasFlow(u, DataFlow::exprNode(e))
-select p, "$@ flows to here and is used in a path.", u, "User-provided value"
+  sink.getNode().asExpr() = p.getInput() and
+  conf.hasFlowPath(source, sink)
+select p, source, sink, "$@ flows to here and is used in a path.", source.getNode(),
+  "User-provided value"

java/ql/src/Security/CWE/CWE-022/TaintedPathLocal.ql

@@ -1,7 +1,7 @@
 /**
  * @name Local-user-controlled data in path expression
  * @description Accessing paths influenced by users can allow an attacker to access unexpected resources.
- * @kind problem
+ * @kind path-problem
  * @problem.severity recommendation
  * @precision medium
  * @id java/path-injection-local
@@ -15,6 +15,7 @@
 import java
 import semmle.code.java.dataflow.FlowSources
 import PathsCommon
+import DataFlow::PathGraph
 
 class TaintedPathLocalConfig extends TaintTracking::Configuration {
   TaintedPathLocalConfig() { this = "TaintedPathLocalConfig" }
@@ -24,9 +25,13 @@ class TaintedPathLocalConfig extends TaintTracking::Configuration {
   override predicate isSink(DataFlow::Node sink) { sink.asExpr() = any(PathCreation p).getInput() }
 }
 
-from LocalUserInput u, PathCreation p, Expr e, TaintedPathLocalConfig conf
+from
+  DataFlow::PathNode source, DataFlow::PathNode sink, PathCreation p, Expr e,
+  TaintedPathLocalConfig conf
 where
+  e = sink.getNode().asExpr() and
   e = p.getInput() and
-  conf.hasFlow(u, DataFlow::exprNode(e)) and
+  conf.hasFlowPath(source, sink) and
   not guarded(e)
-select p, "$@ flows to here and is used in a path.", u, "User-provided value"
+select p, source, sink, "$@ flows to here and is used in a path.", source.getNode(),
+  "User-provided value"

java/ql/src/Security/CWE/CWE-022/ZipSlip.ql

@@ -3,7 +3,7 @@
  * @description Extracting files from a malicious archive without validating that the
  *              destination file path is within the destination directory can cause files outside
  *              the destination directory to be overwritten.
- * @kind problem
+ * @kind path-problem
  * @id java/zipslip
  * @problem.severity error
  * @precision high
@@ -16,6 +16,7 @@ import semmle.code.java.controlflow.Guards
 import semmle.code.java.dataflow.SSA
 import semmle.code.java.dataflow.TaintTracking
 import DataFlow
+import PathGraph
 
 /**
  * A method that returns the name of an archive entry.
@@ -170,7 +171,8 @@ class ZipSlipConfiguration extends TaintTracking::Configuration {
   }
 }
 
-from Node source, Node sink
-where any(ZipSlipConfiguration c).hasFlow(source, sink)
-select source, "Unsanitized archive entry, which may contain '..', is used in a $@.", sink,
+from PathNode source, PathNode sink
+where any(ZipSlipConfiguration c).hasFlowPath(source, sink)
+select source.getNode(), source, sink,
+  "Unsanitized archive entry, which may contain '..', is used in a $@.", sink.getNode(),
   "file system operation"

java/ql/src/Security/CWE/CWE-078/ExecCommon.qll

@@ -20,8 +20,8 @@ private class RemoteUserInputToArgumentToExecFlowConfig extends TaintTracking::C
  * so that it can be excluded from `ExecUnescaped.ql` to avoid
  * reporting overlapping results.
  */
-predicate execTainted(RemoteUserInput source, ArgumentToExec execArg) {
+predicate execTainted(DataFlow::PathNode source, DataFlow::PathNode sink, ArgumentToExec execArg) {
   exists(RemoteUserInputToArgumentToExecFlowConfig conf |
-    conf.hasFlow(source, DataFlow::exprNode(execArg))
+    conf.hasFlowPath(source, sink) and sink.getNode() = DataFlow::exprNode(execArg)
   )
 }

java/ql/src/Security/CWE/CWE-078/ExecTainted.ql

@@ -2,7 +2,7 @@
  * @name Uncontrolled command line
  * @description Using externally controlled strings in a command line is vulnerable to malicious
  *              changes in the strings.
- * @kind problem
+ * @kind path-problem
  * @problem.severity error
  * @precision high
  * @id java/command-line-injection
@@ -15,7 +15,9 @@ import semmle.code.java.Expr
 import semmle.code.java.dataflow.FlowSources
 import semmle.code.java.security.ExternalProcess
 import ExecCommon
+import DataFlow::PathGraph
 
-from StringArgumentToExec execArg, RemoteUserInput origin
-where execTainted(origin, execArg)
-select execArg, "$@ flows to here and is used in a command.", origin, "User-provided value"
+from DataFlow::PathNode source, DataFlow::PathNode sink, StringArgumentToExec execArg
+where execTainted(source, sink, execArg)
+select execArg, source, sink, "$@ flows to here and is used in a command.", source.getNode(),
+  "User-provided value"

java/ql/src/Security/CWE/CWE-078/ExecTaintedLocal.ql

@@ -2,7 +2,7 @@
  * @name Local-user-controlled command line
  * @description Using externally controlled strings in a command line is vulnerable to malicious
  *              changes in the strings.
- * @kind problem
+ * @kind path-problem
  * @problem.severity recommendation
  * @precision medium
  * @id java/command-line-injection-local
@@ -14,6 +14,7 @@
 import semmle.code.java.Expr
 import semmle.code.java.dataflow.FlowSources
 import semmle.code.java.security.ExternalProcess
+import DataFlow::PathGraph
 
 class LocalUserInputToArgumentToExecFlowConfig extends TaintTracking::Configuration {
   LocalUserInputToArgumentToExecFlowConfig() { this = "LocalUserInputToArgumentToExecFlowConfig" }
@@ -28,6 +29,8 @@ class LocalUserInputToArgumentToExecFlowConfig extends TaintTracking::Configurat
 }
 
 from
-  StringArgumentToExec execArg, LocalUserInput origin, LocalUserInputToArgumentToExecFlowConfig conf
-where conf.hasFlow(origin, DataFlow::exprNode(execArg))
-select execArg, "$@ flows to here and is used in a command.", origin, "User-provided value"
+  DataFlow::PathNode source, DataFlow::PathNode sink, StringArgumentToExec execArg,
+  LocalUserInputToArgumentToExecFlowConfig conf
+where conf.hasFlowPath(source, sink) and sink.getNode().asExpr() = execArg
+select execArg, source, sink, "$@ flows to here and is used in a command.", source.getNode(),
+  "User-provided value"

java/ql/src/Security/CWE/CWE-078/ExecUnescaped.ql

@@ -47,5 +47,5 @@ predicate builtFromUncontrolledConcat(Expr expr) {
 from StringArgumentToExec argument
 where
   builtFromUncontrolledConcat(argument) and
-  not execTainted(_, argument)
+  not execTainted(_, _, argument)
 select argument, "Command line is built with string concatenation."

java/ql/src/Security/CWE/CWE-079/XSS.ql

@@ -2,7 +2,7 @@
  * @name Cross-site scripting
  * @description Writing user input directly to a web page
  *              allows for a cross-site scripting vulnerability.
- * @kind problem
+ * @kind path-problem
  * @problem.severity error
  * @precision high
  * @id java/xss
@@ -13,6 +13,7 @@
 import java
 import semmle.code.java.dataflow.FlowSources
 import semmle.code.java.security.XSS
+import DataFlow2::PathGraph
 
 class XSSConfig extends TaintTracking::Configuration2 {
   XSSConfig() { this = "XSSConfig" }
@@ -26,6 +27,7 @@ class XSSConfig extends TaintTracking::Configuration2 {
   }
 }
 
-from XssSink sink, RemoteUserInput source, XSSConfig conf
-where conf.hasFlow(source, sink)
-select sink, "Cross-site scripting vulnerability due to $@.", source, "user-provided value"
+from DataFlow2::PathNode source, DataFlow2::PathNode sink, XSSConfig conf
+where conf.hasFlowPath(source, sink)
+select sink.getNode(), source, sink, "Cross-site scripting vulnerability due to $@.",
+  source.getNode(), "user-provided value"

java/ql/src/Security/CWE/CWE-079/XSSLocal.ql

@@ -2,7 +2,7 @@
  * @name Cross-site scripting from local source
  * @description Writing user input directly to a web page
  *              allows for a cross-site scripting vulnerability.
- * @kind problem
+ * @kind path-problem
  * @problem.severity recommendation
  * @precision medium
  * @id java/xss-local
@@ -13,6 +13,7 @@
 import java
 import semmle.code.java.dataflow.FlowSources
 import semmle.code.java.security.XSS
+import DataFlow2::PathGraph
 
 class XSSLocalConfig extends TaintTracking::Configuration2 {
   XSSLocalConfig() { this = "XSSLocalConfig" }
@@ -22,6 +23,7 @@ class XSSLocalConfig extends TaintTracking::Configuration2 {
   override predicate isSink(DataFlow::Node sink) { sink instanceof XssSink }
 }
 
-from XssSink sink, LocalUserInput source, XSSLocalConfig conf
-where conf.hasFlow(source, sink)
-select sink, "Cross-site scripting vulnerability due to $@.", source, "user-provided value"
+from DataFlow2::PathNode source, DataFlow2::PathNode sink, XSSLocalConfig conf
+where conf.hasFlowPath(source, sink)
+select sink.getNode(), source, sink, "Cross-site scripting vulnerability due to $@.",
+  source.getNode(), "user-provided value"

java/ql/src/Security/CWE/CWE-089/SqlInjectionLib.qll

@@ -62,6 +62,8 @@ private class QueryInjectionFlowConfig extends TaintTracking::Configuration {
  * Implementation of `SqlTainted.ql`. This is extracted to a QLL so that it
  * can be excluded from `SqlUnescaped.ql` to avoid overlapping results.
  */
-predicate queryTaintedBy(QueryInjectionSink query, RemoteUserInput source) {
-  exists(QueryInjectionFlowConfig conf | conf.hasFlow(source, query))
+predicate queryTaintedBy(
+  QueryInjectionSink query, DataFlow::PathNode source, DataFlow::PathNode sink
+) {
+  exists(QueryInjectionFlowConfig conf | conf.hasFlowPath(source, sink) and sink.getNode() = query)
 }

java/ql/src/Security/CWE/CWE-089/SqlTainted.ql

@@ -2,7 +2,7 @@
  * @name Query built from user-controlled sources
  * @description Building a SQL or Java Persistence query from user-controlled sources is vulnerable to insertion of
  *              malicious code by the user.
- * @kind problem
+ * @kind path-problem
  * @problem.severity error
  * @precision high
  * @id java/sql-injection
@@ -13,7 +13,8 @@
 import semmle.code.java.Expr
 import semmle.code.java.dataflow.FlowSources
 import SqlInjectionLib
+import DataFlow::PathGraph
 
-from QueryInjectionSink query, RemoteUserInput source
-where queryTaintedBy(query, source)
-select query, "Query might include code from $@.", source, "this user input"
+from QueryInjectionSink query, DataFlow::PathNode source, DataFlow::PathNode sink
+where queryTaintedBy(query, source, sink)
+select query, source, sink, "Query might include code from $@.", source.getNode(), "this user input"

java/ql/src/Security/CWE/CWE-089/SqlTaintedLocal.ql

@@ -2,7 +2,7 @@
  * @name Query built from local-user-controlled sources
  * @description Building a SQL or Java Persistence query from user-controlled sources is vulnerable to insertion of
  *              malicious code by the user.
- * @kind problem
+ * @kind path-problem
  * @problem.severity recommendation
  * @precision medium
  * @id java/sql-injection-local
@@ -13,6 +13,7 @@
 import semmle.code.java.Expr
 import semmle.code.java.dataflow.FlowSources
 import SqlInjectionLib
+import DataFlow::PathGraph
 
 class LocalUserInputToQueryInjectionFlowConfig extends TaintTracking::Configuration {
   LocalUserInputToQueryInjectionFlowConfig() { this = "LocalUserInputToQueryInjectionFlowConfig" }
@@ -26,6 +27,8 @@ class LocalUserInputToQueryInjectionFlowConfig extends TaintTracking::Configurat
   }
 }
 
-from QueryInjectionSink query, LocalUserInput source, LocalUserInputToQueryInjectionFlowConfig conf
-where conf.hasFlow(source, query)
-select query, "Query might include code from $@.", source, "this user input"
+from
+  DataFlow::PathNode source, DataFlow::PathNode sink, LocalUserInputToQueryInjectionFlowConfig conf
+where conf.hasFlowPath(source, sink)
+select sink.getNode(), source, sink, "Query might include code from $@.", source.getNode(),
+  "this user input"

java/ql/src/Security/CWE/CWE-089/SqlUnescaped.ql

@@ -46,6 +46,6 @@ where
       conf.hasFlow(DataFlow::exprNode(sbv.getToStringCall()), query)
     )
   ) and
-  not queryTaintedBy(query, _)
+  not queryTaintedBy(query, _, _)
 select query, "Query might not neutralize special characters in $@.", uncontrolled,
   "this expression"

java/ql/src/Security/CWE/CWE-113/ResponseSplitting.ql

@@ -2,7 +2,7 @@
  * @name HTTP response splitting
  * @description Writing user input directly to an HTTP header
  *              makes code vulnerable to attack by header splitting.
- * @kind problem
+ * @kind path-problem
  * @problem.severity error
  * @precision high
  * @id java/http-response-splitting
@@ -12,6 +12,7 @@
 
 import java
 import ResponseSplitting
+import DataFlow::PathGraph
 
 class ResponseSplittingConfig extends TaintTracking::Configuration {
   ResponseSplittingConfig() { this = "ResponseSplittingConfig" }
@@ -24,6 +25,7 @@ class ResponseSplittingConfig extends TaintTracking::Configuration {
   override predicate isSink(DataFlow::Node sink) { sink instanceof HeaderSplittingSink }
 }
 
-from HeaderSplittingSink sink, RemoteUserInput source, ResponseSplittingConfig conf
-where conf.hasFlow(source, sink)
-select sink, "Response-splitting vulnerability due to this $@.", source, "user-provided value"
+from DataFlow::PathNode source, DataFlow::PathNode sink, ResponseSplittingConfig conf
+where conf.hasFlowPath(source, sink)
+select sink.getNode(), source, sink, "Response-splitting vulnerability due to this $@.",
+  source.getNode(), "user-provided value"

java/ql/src/Security/CWE/CWE-113/ResponseSplittingLocal.ql

@@ -2,7 +2,7 @@
  * @name HTTP response splitting from local source
  * @description Writing user input directly to an HTTP header
  *              makes code vulnerable to attack by header splitting.
- * @kind problem
+ * @kind path-problem
  * @problem.severity recommendation
  * @precision medium
  * @id java/http-response-splitting-local
@@ -13,6 +13,7 @@
 import java
 import semmle.code.java.dataflow.FlowSources
 import ResponseSplitting
+import DataFlow::PathGraph
 
 class ResponseSplittingLocalConfig extends TaintTracking::Configuration {
   ResponseSplittingLocalConfig() { this = "ResponseSplittingLocalConfig" }
@@ -22,6 +23,7 @@ class ResponseSplittingLocalConfig extends TaintTracking::Configuration {
   override predicate isSink(DataFlow::Node sink) { sink instanceof HeaderSplittingSink }
 }
 
-from HeaderSplittingSink sink, LocalUserInput source, ResponseSplittingLocalConfig conf
-where conf.hasFlow(source, sink)
-select sink, "Response-splitting vulnerability due to this $@.", source, "user-provided value"
+from DataFlow::PathNode source, DataFlow::PathNode sink, ResponseSplittingLocalConfig conf
+where conf.hasFlowPath(source, sink)
+select sink.getNode(), source, sink, "Response-splitting vulnerability due to this $@.",
+  source.getNode(), "user-provided value"