Merge branch 'master' into node-js-classification

Author: Esben Sparre Andreasen

change-notes/1.23/analysis-javascript.md

@@ -23,8 +23,12 @@
 | Incomplete string escaping or encoding (`js/incomplete-sanitization`) | Fewer false-positive results | This rule now recognizes additional ways delimiters can be stripped away. |
 | Client-side cross-site scripting (`js/xss`) | More results | More potential vulnerabilities involving functions that manipulate DOM attributes are now recognized. |
 | Code injection (`js/code-injection`) | More results | More potential vulnerabilities involving functions that manipulate DOM event handler attributes are now recognized. |
+| Hard-coded credentials (`js/hardcoded-credentials`) | Fewer false-positive results | This rule now flags fewer password examples. |
+| Incorrect suffix check (`js/incorrect-suffix-check`) | Fewer false-positive results | The query recognizes valid checks in more cases. 
+| Network data written to file (`js/http-to-file-access`) | Fewer false-positive results | This query has been renamed to better match its intended purpose, and now only considers network data untrusted. | 
+| Password in configuration file (`js/password-in-configuration-file`) | Fewer false-positive results | This rule now flags fewer password examples. |
 | Prototype pollution (`js/prototype-pollution`) | More results | The query now highlights vulnerable uses of jQuery and Angular, and the results are shown on LGTM by default. |
-| Incorrect suffix check (`js/incorrect-suffix-check`) | Fewer false-positive results | The query recognizes valid checks in more cases. |
+| Uncontrolled command line (`js/command-line-injection`) | More results | This query now treats responses from servers as untrusted. |
 
 ## Changes to QL libraries
 

change-notes/1.23/extractor-javascript.md

@@ -5,6 +5,6 @@
 ## Changes to code extraction
 
 * Asynchronous generator methods are now parsed correctly and no longer cause a spurious syntax error.
-
 * Recognition of CommonJS modules has improved. As a result, some files that were previously extracted as
   global scripts are now extracted as modules.
+* Top-level `await` is now supported.

cpp/ql/src/semmle/code/cpp/Variable.qll

@@ -315,7 +315,7 @@ class ParameterDeclarationEntry extends VariableDeclarationEntry {
  *   static int c;
  * }
  * ```
- * 
+ *
  * Local variables can be static; use the `isStatic` member predicate to
  * detect those.
  */
@@ -343,7 +343,7 @@ deprecated class StackVariable extends Variable {
  *   static int c;
  * }
  * ```
- * 
+ *
  * Local variables can be static; use the `isStatic` member predicate to detect
  * those.
  *
@@ -512,9 +512,9 @@ class TemplateVariable extends Variable {
  * void myTemplateFunction() {
  *   T b;
  * }
- * 
+ *
  * ...
- * 
+ *
  * myTemplateFunction<int>();
  * ```
  */

cpp/ql/src/semmle/code/cpp/commons/CommonType.qll

@@ -174,9 +174,7 @@ class MicrosoftInt64Type extends IntegralType {
  * `__builtin_va_copy` and `__builtin_va_arg` expressions.
  */
 class BuiltInVarArgsList extends Type {
-  BuiltInVarArgsList() {
-    this.hasName("__builtin_va_list")
-  }
+  BuiltInVarArgsList() { this.hasName("__builtin_va_list") }
 
   override string getCanonicalQLClass() { result = "BuiltInVarArgsList" }
 }

cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl.qll

@@ -1618,6 +1618,9 @@ abstract class PathNode extends TPathNode {
   /** Gets a successor of this node, if any. */
   abstract PathNode getASuccessor();
 
+  /** Holds if this node is a source. */
+  abstract predicate isSource();
+
   private string ppAp() {
     this instanceof PathNodeSink and result = ""
     or
@@ -1683,12 +1686,6 @@ private class PathNodeMid extends PathNode, TPathNodeMid {
     // an intermediate step to another intermediate node
     result = getSuccMid()
     or
-    // a final step to a sink via one or more local steps
-    localFlowStepPlus(node, result.getNode(), _, config) and
-    ap instanceof AccessPathNil and
-    result instanceof PathNodeSink and
-    result.getConfiguration() = unbind(this.getConfiguration())
-    or
     // a final step to a sink via zero steps means we merge the last two steps to prevent trivial-looking edges
     exists(PathNodeMid mid |
       mid = getSuccMid() and
@@ -1697,23 +1694,12 @@ private class PathNodeMid extends PathNode, TPathNodeMid {
       result instanceof PathNodeSink and
       result.getConfiguration() = unbind(mid.getConfiguration())
     )
-    or
-    // a direct step from a source to a sink if a node is both
-    this instanceof PathNodeSource and
-    result instanceof PathNodeSink and
-    this.getNode() = result.getNode() and
-    result.getConfiguration() = unbind(this.getConfiguration())
   }
-}
 
-/**
- * A flow graph node corresponding to a source.
- */
-private class PathNodeSource extends PathNodeMid {
-  PathNodeSource() {
-    getConfiguration().isSource(getNode()) and
-    getCallContext() instanceof CallContextAny and
-    getAp() instanceof AccessPathNil
+  override predicate isSource() {
+    config.isSource(node) and
+    cc instanceof CallContextAny and
+    ap instanceof AccessPathNil
   }
 }
 
@@ -1733,6 +1719,8 @@ private class PathNodeSink extends PathNode, TPathNodeSink {
   override Configuration getConfiguration() { result = config }
 
   override PathNode getASuccessor() { none() }
+
+  override predicate isSource() { config.isSource(node) }
 }
 
 /**
@@ -1967,12 +1955,12 @@ private predicate valuePathThroughCallable(PathNodeMid mid, OutNode out, CallCon
  * sinks.
  */
 private predicate flowsTo(
-  PathNodeSource flowsource, PathNodeSink flowsink, Node source, Node sink,
-  Configuration configuration
+  PathNode flowsource, PathNodeSink flowsink, Node source, Node sink, Configuration configuration
 ) {
+  flowsource.isSource() and
   flowsource.getConfiguration() = configuration and
   flowsource.getNode() = source and
-  pathSuccPlus(flowsource, flowsink) and
+  (flowsource = flowsink or pathSuccPlus(flowsource, flowsink)) and
   flowsink.getNode() = sink
 }
 

cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl2.qll

@@ -1618,6 +1618,9 @@ abstract class PathNode extends TPathNode {
   /** Gets a successor of this node, if any. */
   abstract PathNode getASuccessor();
 
+  /** Holds if this node is a source. */
+  abstract predicate isSource();
+
   private string ppAp() {
     this instanceof PathNodeSink and result = ""
     or
@@ -1683,12 +1686,6 @@ private class PathNodeMid extends PathNode, TPathNodeMid {
     // an intermediate step to another intermediate node
     result = getSuccMid()
     or
-    // a final step to a sink via one or more local steps
-    localFlowStepPlus(node, result.getNode(), _, config) and
-    ap instanceof AccessPathNil and
-    result instanceof PathNodeSink and
-    result.getConfiguration() = unbind(this.getConfiguration())
-    or
     // a final step to a sink via zero steps means we merge the last two steps to prevent trivial-looking edges
     exists(PathNodeMid mid |
       mid = getSuccMid() and
@@ -1697,23 +1694,12 @@ private class PathNodeMid extends PathNode, TPathNodeMid {
       result instanceof PathNodeSink and
       result.getConfiguration() = unbind(mid.getConfiguration())
     )
-    or
-    // a direct step from a source to a sink if a node is both
-    this instanceof PathNodeSource and
-    result instanceof PathNodeSink and
-    this.getNode() = result.getNode() and
-    result.getConfiguration() = unbind(this.getConfiguration())
   }
-}
 
-/**
- * A flow graph node corresponding to a source.
- */
-private class PathNodeSource extends PathNodeMid {
-  PathNodeSource() {
-    getConfiguration().isSource(getNode()) and
-    getCallContext() instanceof CallContextAny and
-    getAp() instanceof AccessPathNil
+  override predicate isSource() {
+    config.isSource(node) and
+    cc instanceof CallContextAny and
+    ap instanceof AccessPathNil
   }
 }
 
@@ -1733,6 +1719,8 @@ private class PathNodeSink extends PathNode, TPathNodeSink {
   override Configuration getConfiguration() { result = config }
 
   override PathNode getASuccessor() { none() }
+
+  override predicate isSource() { config.isSource(node) }
 }
 
 /**
@@ -1967,12 +1955,12 @@ private predicate valuePathThroughCallable(PathNodeMid mid, OutNode out, CallCon
  * sinks.
  */
 private predicate flowsTo(
-  PathNodeSource flowsource, PathNodeSink flowsink, Node source, Node sink,
-  Configuration configuration
+  PathNode flowsource, PathNodeSink flowsink, Node source, Node sink, Configuration configuration
 ) {
+  flowsource.isSource() and
   flowsource.getConfiguration() = configuration and
   flowsource.getNode() = source and
-  pathSuccPlus(flowsource, flowsink) and
+  (flowsource = flowsink or pathSuccPlus(flowsource, flowsink)) and
   flowsink.getNode() = sink
 }
 

cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl3.qll

@@ -1618,6 +1618,9 @@ abstract class PathNode extends TPathNode {
   /** Gets a successor of this node, if any. */
   abstract PathNode getASuccessor();
 
+  /** Holds if this node is a source. */
+  abstract predicate isSource();
+
   private string ppAp() {
     this instanceof PathNodeSink and result = ""
     or
@@ -1683,12 +1686,6 @@ private class PathNodeMid extends PathNode, TPathNodeMid {
     // an intermediate step to another intermediate node
     result = getSuccMid()
     or
-    // a final step to a sink via one or more local steps
-    localFlowStepPlus(node, result.getNode(), _, config) and
-    ap instanceof AccessPathNil and
-    result instanceof PathNodeSink and
-    result.getConfiguration() = unbind(this.getConfiguration())
-    or
     // a final step to a sink via zero steps means we merge the last two steps to prevent trivial-looking edges
     exists(PathNodeMid mid |
       mid = getSuccMid() and
@@ -1697,23 +1694,12 @@ private class PathNodeMid extends PathNode, TPathNodeMid {
       result instanceof PathNodeSink and
       result.getConfiguration() = unbind(mid.getConfiguration())
     )
-    or
-    // a direct step from a source to a sink if a node is both
-    this instanceof PathNodeSource and
-    result instanceof PathNodeSink and
-    this.getNode() = result.getNode() and
-    result.getConfiguration() = unbind(this.getConfiguration())
   }
-}
 
-/**
- * A flow graph node corresponding to a source.
- */
-private class PathNodeSource extends PathNodeMid {
-  PathNodeSource() {
-    getConfiguration().isSource(getNode()) and
-    getCallContext() instanceof CallContextAny and
-    getAp() instanceof AccessPathNil
+  override predicate isSource() {
+    config.isSource(node) and
+    cc instanceof CallContextAny and
+    ap instanceof AccessPathNil
   }
 }
 
@@ -1733,6 +1719,8 @@ private class PathNodeSink extends PathNode, TPathNodeSink {
   override Configuration getConfiguration() { result = config }
 
   override PathNode getASuccessor() { none() }
+
+  override predicate isSource() { config.isSource(node) }
 }
 
 /**
@@ -1967,12 +1955,12 @@ private predicate valuePathThroughCallable(PathNodeMid mid, OutNode out, CallCon
  * sinks.
  */
 private predicate flowsTo(
-  PathNodeSource flowsource, PathNodeSink flowsink, Node source, Node sink,
-  Configuration configuration
+  PathNode flowsource, PathNodeSink flowsink, Node source, Node sink, Configuration configuration
 ) {
+  flowsource.isSource() and
   flowsource.getConfiguration() = configuration and
   flowsource.getNode() = source and
-  pathSuccPlus(flowsource, flowsink) and
+  (flowsource = flowsink or pathSuccPlus(flowsource, flowsink)) and
   flowsink.getNode() = sink
 }
 

cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl4.qll

@@ -1618,6 +1618,9 @@ abstract class PathNode extends TPathNode {
   /** Gets a successor of this node, if any. */
   abstract PathNode getASuccessor();
 
+  /** Holds if this node is a source. */
+  abstract predicate isSource();
+
   private string ppAp() {
     this instanceof PathNodeSink and result = ""
     or
@@ -1683,12 +1686,6 @@ private class PathNodeMid extends PathNode, TPathNodeMid {
     // an intermediate step to another intermediate node
     result = getSuccMid()
     or
-    // a final step to a sink via one or more local steps
-    localFlowStepPlus(node, result.getNode(), _, config) and
-    ap instanceof AccessPathNil and
-    result instanceof PathNodeSink and
-    result.getConfiguration() = unbind(this.getConfiguration())
-    or
     // a final step to a sink via zero steps means we merge the last two steps to prevent trivial-looking edges
     exists(PathNodeMid mid |
       mid = getSuccMid() and
@@ -1697,23 +1694,12 @@ private class PathNodeMid extends PathNode, TPathNodeMid {
       result instanceof PathNodeSink and
       result.getConfiguration() = unbind(mid.getConfiguration())
     )
-    or
-    // a direct step from a source to a sink if a node is both
-    this instanceof PathNodeSource and
-    result instanceof PathNodeSink and
-    this.getNode() = result.getNode() and
-    result.getConfiguration() = unbind(this.getConfiguration())
   }
-}
 
-/**
- * A flow graph node corresponding to a source.
- */
-private class PathNodeSource extends PathNodeMid {
-  PathNodeSource() {
-    getConfiguration().isSource(getNode()) and
-    getCallContext() instanceof CallContextAny and
-    getAp() instanceof AccessPathNil
+  override predicate isSource() {
+    config.isSource(node) and
+    cc instanceof CallContextAny and
+    ap instanceof AccessPathNil
   }
 }
 
@@ -1733,6 +1719,8 @@ private class PathNodeSink extends PathNode, TPathNodeSink {
   override Configuration getConfiguration() { result = config }
 
   override PathNode getASuccessor() { none() }
+
+  override predicate isSource() { config.isSource(node) }
 }
 
 /**
@@ -1967,12 +1955,12 @@ private predicate valuePathThroughCallable(PathNodeMid mid, OutNode out, CallCon
  * sinks.
  */
 private predicate flowsTo(
-  PathNodeSource flowsource, PathNodeSink flowsink, Node source, Node sink,
-  Configuration configuration
+  PathNode flowsource, PathNodeSink flowsink, Node source, Node sink, Configuration configuration
 ) {
+  flowsource.isSource() and
   flowsource.getConfiguration() = configuration and
   flowsource.getNode() = source and
-  pathSuccPlus(flowsource, flowsink) and
+  (flowsource = flowsink or pathSuccPlus(flowsource, flowsink)) and
   flowsink.getNode() = sink
 }