JavaScript: Convert security queries to path queries where applicable.

change-notes/1.19/analysis-javascript.md

@@ -13,6 +13,8 @@
 
 * Type inference for function calls has been improved. This may give additional results for queries that rely on type inference.
 
+* Where applicable, path explanations have been added to the security queries.
+
 ## New queries
 
 | **Query**                                     | **Tags**                                             | **Purpose**                                                                                                                                                                 |

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

@@ -2,7 +2,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 js/path-injection
@@ -15,9 +15,10 @@
  */
 
 import javascript
-import semmle.javascript.security.dataflow.RemoteFlowSources
 import semmle.javascript.security.dataflow.TaintedPath::TaintedPath
+import DataFlow::PathGraph
 
-from Configuration cfg, DataFlow::Node source, DataFlow::Node sink
-where cfg.hasFlow(source, sink)
-select sink, "This path depends on $@.", source, "a user-provided value"
+from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
+where cfg.hasPathFlow(source, sink)
+select sink.getNode(), source, sink, "This path depends on $@.",
+       source.getNode(), "a user-provided value"

javascript/ql/src/Security/CWE-078/CommandInjection.ql

@@ -2,7 +2,7 @@
  * @name Uncontrolled command line
  * @description Using externally controlled strings in a command line may allow a malicious
  *              user to change the meaning of the command.
- * @kind problem
+ * @kind path-problem
  * @problem.severity error
  * @precision high
  * @id js/command-line-injection
@@ -14,11 +14,13 @@
 
 import javascript
 import semmle.javascript.security.dataflow.CommandInjection::CommandInjection
+import DataFlow::PathGraph
 
-from Configuration cfg, DataFlow::Node source, DataFlow::Node sink, DataFlow::Node highlight
-where cfg.hasFlow(source, sink) and
-      if cfg.isSinkWithHighlight(sink, _) then
-        cfg.isSinkWithHighlight(sink, highlight)
+from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink, DataFlow::Node highlight
+where cfg.hasPathFlow(source, sink) and
+      if cfg.isSinkWithHighlight(sink.getNode(), _) then
+        cfg.isSinkWithHighlight(sink.getNode(), highlight)
       else
-        highlight = sink
-select highlight, "This command depends on $@.", source, "a user-provided value"
+        highlight = sink.getNode()
+select highlight, source, sink, "This command depends on $@.",
+       source.getNode(), "a user-provided value"

javascript/ql/src/Security/CWE-079/ReflectedXss.ql

@@ -2,7 +2,7 @@
  * @name Reflected cross-site scripting
  * @description Writing user input directly to an HTTP response allows for
  *              a cross-site scripting vulnerability.
- * @kind problem
+ * @kind path-problem
  * @problem.severity error
  * @precision high
  * @id js/reflected-xss
@@ -13,8 +13,9 @@
 
 import javascript
 import semmle.javascript.security.dataflow.ReflectedXss::ReflectedXss
+import DataFlow::PathGraph
 
-from Configuration xss, DataFlow::Node source, DataFlow::Node sink
-where xss.hasFlow(source, sink)
-select sink, "Cross-site scripting vulnerability due to $@.",
-       source, "user-provided value"
+from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
+where cfg.hasPathFlow(source, sink)
+select sink.getNode(), source, sink, "Cross-site scripting vulnerability due to $@.",
+       source.getNode(), "user-provided value"

javascript/ql/src/Security/CWE-079/StoredXss.ql

@@ -2,7 +2,7 @@
  * @name Stored cross-site scripting
  * @description Using uncontrolled stored values in HTML allows for
  *              a stored cross-site scripting vulnerability.
- * @kind problem
+ * @kind path-problem
  * @problem.severity error
  * @precision high
  * @id js/stored-xss
@@ -13,8 +13,9 @@
 
 import javascript
 import semmle.javascript.security.dataflow.StoredXss::StoredXss
+import DataFlow::PathGraph
 
-from Configuration xss, DataFlow::Node source, DataFlow::Node sink
-where xss.hasFlow(source, sink)
-select sink, "Stored cross-site scripting vulnerability due to $@.",
-       source, "stored value"
+from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
+where cfg.hasPathFlow(source, sink)
+select sink.getNode(), source, sink, "Stored cross-site scripting vulnerability due to $@.",
+       source.getNode(), "stored value"

javascript/ql/src/Security/CWE-079/Xss.ql

@@ -2,7 +2,7 @@
  * @name Client side cross-site scripting
  * @description Writing user input directly to the DOM allows for
  *              a cross-site scripting vulnerability.
- * @kind problem
+ * @kind path-problem
  * @problem.severity error
  * @precision high
  * @id js/xss
@@ -13,8 +13,9 @@
 
 import javascript
 import semmle.javascript.security.dataflow.DomBasedXss::DomBasedXss
+import DataFlow::PathGraph
 
-from Configuration xss, DataFlow::Node source, Sink sink
-where xss.hasFlow(source, sink)
-select sink, sink.getVulnerabilityKind() + " vulnerability due to $@.",
-       source, "user-provided value"
+from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
+where cfg.hasPathFlow(source, sink)
+select sink.getNode(), source, sink, sink.getNode().(Sink).getVulnerabilityKind() + " vulnerability due to $@.",
+       source.getNode(), "user-provided value"

javascript/ql/src/Security/CWE-089/SqlInjection.ql

@@ -2,7 +2,7 @@
 * @name Database query built from user-controlled sources
 * @description Building a database 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 js/sql-injection
@@ -13,16 +13,11 @@
 import javascript
 import semmle.javascript.security.dataflow.SqlInjection
 import semmle.javascript.security.dataflow.NosqlInjection
+import DataFlow::PathGraph
 
-predicate sqlInjection(DataFlow::Node source, DataFlow::Node sink) {
-  any(SqlInjection::Configuration cfg).hasFlow(source, sink)
-}
-
-predicate nosqlInjection(DataFlow::Node source, DataFlow::Node sink) {
-  any(NosqlInjection::Configuration cfg).hasFlow(source, sink)
-}
-
-from DataFlow::Node source, DataFlow::Node sink
-where sqlInjection(source, sink) or
-      nosqlInjection(source, sink)
-select sink, "This query depends on $@.", source, "a user-provided value"
+from DataFlow::Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
+where (cfg instanceof SqlInjection::Configuration or
+       cfg instanceof NosqlInjection::Configuration) and
+      cfg.hasPathFlow(source, sink)
+select sink.getNode(), source, sink, "This query depends on $@.",
+       source.getNode(), "a user-provided value"

javascript/ql/src/Security/CWE-094/CodeInjection.ql

@@ -2,7 +2,7 @@
  * @name Code injection
  * @description Interpreting unsanitized user input as code allows a malicious user arbitrary
  *              code execution.
- * @kind problem
+ * @kind path-problem
  * @problem.severity error
  * @precision high
  * @id js/code-injection
@@ -14,7 +14,9 @@
 
 import javascript
 import semmle.javascript.security.dataflow.CodeInjection::CodeInjection
+import DataFlow::PathGraph
 
-from Configuration codeInjection, DataFlow::Node source, DataFlow::Node sink
-where codeInjection.hasFlow(source, sink)
-select sink, "$@ flows to here and is interpreted as code.", source, "User-provided value"
+from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
+where cfg.hasPathFlow(source, sink)
+select sink.getNode(), source, sink, "$@ flows to here and is interpreted as code.",
+       source.getNode(), "User-provided value"

javascript/ql/src/Security/CWE-134/TaintedFormatString.ql

@@ -1,7 +1,7 @@
 /**
  * @name Use of externally-controlled format string
  * @description Using external input in format strings can lead to garbled output.
- * @kind problem
+ * @kind path-problem
  * @problem.severity warning
  * @precision high
  * @id js/tainted-format-string
@@ -11,7 +11,9 @@
 
 import javascript
 import semmle.javascript.security.dataflow.TaintedFormatString::TaintedFormatString
+import DataFlow::PathGraph
 
-from Configuration c, DataFlow::Node source, DataFlow::Node sink
-where c.hasFlow(source, sink)
-select sink, "$@ flows here and is used in a format string.", source, "User-provided value"
+from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
+where cfg.hasPathFlow(source, sink)
+select sink.getNode(), source, sink, "$@ flows here and is used in a format string.",
+       source.getNode(), "User-provided value"

javascript/ql/src/Security/CWE-200/FileAccessToHttp.ql

@@ -1,16 +1,18 @@
 /**
  * @name File data in outbound network request
  * @description Directly sending file data in an outbound network request can indicate unauthorized information disclosure.
- * @kind problem
+ * @kind path-problem
  * @problem.severity warning
  * @id js/file-access-to-http
  * @tags security
  *       external/cwe/cwe-200
  */
 
 import javascript
-import semmle.javascript.security.dataflow.FileAccessToHttp
+import semmle.javascript.security.dataflow.FileAccessToHttp::FileAccessToHttp
+import DataFlow::PathGraph
 
-from FileAccessToHttp::Configuration config, DataFlow::Node src, DataFlow::Node sink
-where config.hasFlow (src, sink)
-select sink, "$@ flows directly to outbound network request", src, "File data"
+from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
+where cfg.hasPathFlow(source, sink)
+select sink.getNode(), source, sink, "$@ flows directly to outbound network request",
+       source.getNode(), "File data"

javascript/ql/src/Security/CWE-209/StackTraceExposure.ql

@@ -3,7 +3,7 @@
  * @description Propagating stack trace information to an external user can
  *              unintentionally reveal implementation details that are useful
  *              to an attacker for developing a subsequent exploit.
- * @kind problem
+ * @kind path-problem
  * @problem.severity warning
  * @precision very-high
  * @id js/stack-trace-exposure
@@ -13,8 +13,9 @@
 
 import javascript
 import semmle.javascript.security.dataflow.StackTraceExposure::StackTraceExposure
+import DataFlow::PathGraph
 
-from Configuration cfg, DataFlow::Node source, DataFlow::Node sink
-where cfg.hasFlow(source, sink)
-select sink, "Stack trace information from $@ may be exposed to an external user here.",
-       source, "here"
+from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
+where cfg.hasPathFlow(source, sink)
+select sink.getNode(), source, sink, "Stack trace information from $@ may be exposed to an external user here.",
+       source.getNode(), "here"

javascript/ql/src/Security/CWE-312/CleartextLogging.ql

@@ -2,7 +2,7 @@
  * @name Clear-text logging of sensitive information
  * @description Logging sensitive information without encryption or hashing can
  *              expose it to an attacker.
- * @kind problem
+ * @kind path-problem
  * @problem.severity error
  * @precision high
  * @id js/clear-text-logging
@@ -14,6 +14,7 @@
 
 import javascript
 import semmle.javascript.security.dataflow.CleartextLogging::CleartextLogging
+import DataFlow::PathGraph
 
 /**
  * Holds if `tl` is used in a browser environment.
@@ -31,8 +32,9 @@ predicate inBrowserEnvironment(TopLevel tl) {
   )
 }
 
-from Configuration cfg, Source source, DataFlow::Node sink
-where cfg.hasFlow(source, sink) and
+from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
+where cfg.hasPathFlow(source, sink) and
       // ignore logging to the browser console (even though it is not a good practice)
-      not inBrowserEnvironment(sink.asExpr().getTopLevel())
-select sink, "Sensitive data returned by $@ is logged here.", source, source.describe()
+      not inBrowserEnvironment(sink.getNode().asExpr().getTopLevel())
+select sink.getNode(), source, sink, "Sensitive data returned by $@ is logged here.",
+       source.getNode(), source.getNode().(Source).describe()

javascript/ql/src/Security/CWE-312/CleartextStorage.ql

@@ -2,7 +2,7 @@
  * @name Clear text storage of sensitive information
  * @description Sensitive information stored without encryption or hashing can expose it to an
  *              attacker.
- * @kind problem
+ * @kind path-problem
  * @problem.severity error
  * @precision high
  * @id js/clear-text-storage-of-sensitive-data
@@ -14,7 +14,9 @@
 
 import javascript
 import semmle.javascript.security.dataflow.CleartextStorage::CleartextStorage
+import DataFlow::PathGraph
 
-from Configuration cleartextStorage, Source source, DataFlow::Node sink
-where cleartextStorage.hasFlow(source, sink)
-select sink, "Sensitive data returned by $@ is stored here.", source, source.describe()
+from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
+where cfg.hasPathFlow(source, sink)
+select sink.getNode(), source, sink, "Sensitive data returned by $@ is stored here.",
+       source.getNode(), source.getNode().(Source).describe()

javascript/ql/src/Security/CWE-327/BrokenCryptoAlgorithm.ql

@@ -1,19 +1,21 @@
 /**
  * @name Use of a broken or weak cryptographic algorithm
  * @description Using broken or weak cryptographic algorithms can compromise security.
- * @kind problem
+ * @kind path-problem
  * @problem.severity warning
  * @precision high
  * @id js/weak-cryptographic-algorithm
  * @tags security
  *       external/cwe/cwe-327
  */
+
 import javascript
-import semmle.javascript.security.dataflow.RemoteFlowSources
 import semmle.javascript.security.dataflow.BrokenCryptoAlgorithm::BrokenCryptoAlgorithm
 import semmle.javascript.security.SensitiveActions
+import DataFlow::PathGraph
 
-from Configuration brokenCrypto, Source source, DataFlow::Node sink
-where brokenCrypto.hasFlow(source, sink) and
-      not source.asExpr() instanceof CleartextPasswordExpr // flagged by js/insufficient-password-hash
-select sink, "Sensitive data from $@ is used in a broken or weak cryptographic algorithm.", source , source.describe()
+from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
+where cfg.hasPathFlow(source, sink) and
+      not source.getNode().asExpr() instanceof CleartextPasswordExpr // flagged by js/insufficient-password-hash
+select sink.getNode(), source, sink, "Sensitive data from $@ is used in a broken or weak cryptographic algorithm.",
+       source.getNode(), source.getNode().(Source).describe()

javascript/ql/src/Security/CWE-338/InsecureRandomness.ql

@@ -3,7 +3,7 @@
  * @description Using a cryptographically weak pseudo-random number generator to generate a
  *              security-sensitive value may allow an attacker to predict what value will
  *              be generated.
- * @kind problem
+ * @kind path-problem
  * @problem.severity warning
  * @precision high
  * @id js/insecure-randomness
@@ -12,7 +12,9 @@
  */
 import javascript
 import semmle.javascript.security.dataflow.InsecureRandomness::InsecureRandomness
+import DataFlow::PathGraph
 
-from Configuration cfg, DataFlow::Node source, DataFlow::Node sink
-where cfg.hasFlow(source, sink)
-select sink, "Cryptographically insecure $@ in a security context.", source, "random value"
+from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
+where cfg.hasPathFlow(source, sink)
+select sink.getNode(), source, sink, "Cryptographically insecure $@ in a security context.",
+       source.getNode(), "random value"

javascript/ql/src/Security/CWE-346/CorsMisconfigurationForCredentials.ql

@@ -1,7 +1,7 @@
 /**
  * @name CORS misconfiguration for credentials transfer
  * @description Misconfiguration of CORS HTTP headers allows for leaks of secret credentials.
- * @kind problem
+ * @kind path-problem
  * @problem.severity error
  * @precision high
  * @id js/cors-misconfiguration-for-credentials
@@ -13,9 +13,10 @@
 
 import javascript
 import semmle.javascript.security.dataflow.CorsMisconfigurationForCredentials::CorsMisconfigurationForCredentials
+import DataFlow::PathGraph
 
-from Configuration cfg, DataFlow::Node source, Sink sink
-where cfg.hasFlow(source, sink)
-select sink, "$@ leak vulnerability due to $@.",
-       sink.getCredentialsHeader(), "Credential",
-       source, "a misconfigured CORS header value"
+from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
+where cfg.hasPathFlow(source, sink)
+select sink.getNode(), source, sink, "$@ leak vulnerability due to $@.",
+       sink.getNode().(Sink).getCredentialsHeader(), "Credential",
+       source.getNode(), "a misconfigured CORS header value"