Merge pull request #480 from aschackmull/java/path-problem-conversion

Java: Convert security queries to path-problem.

Author: yh-semmle

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"