Merge remote-tracking branch 'upstream/master' into promiseAll

Author: erik-krogh

change-notes/1.24/analysis-cpp.md

@@ -0,0 +1,20 @@
+# Improvements to C/C++ analysis
+
+The following changes in version 1.24 affect C/C++ analysis in all applications.
+
+## General improvements
+
+## New queries
+
+| **Query**                   | **Tags**  | **Purpose**                                                        |
+|-----------------------------|-----------|--------------------------------------------------------------------|
+
+## Changes to existing queries
+
+| **Query**                  | **Expected impact**    | **Change**                                                       |
+|----------------------------|------------------------|------------------------------------------------------------------|
+| No space for zero terminator (`cpp/no-space-for-terminator`) | Fewer false positive results | This query has been modified to be more conservative when identifying which pointers point to null-terminated strings.  This approach produces fewer, more accurate results. |
+
+## Changes to libraries
+
+* 

change-notes/1.24/analysis-javascript.md

@@ -4,6 +4,7 @@
 
 * Support for the following frameworks and libraries has been improved:
   - [react](https://www.npmjs.com/package/react)
+  - [Handlebars](https://www.npmjs.com/package/handlebars)
 
 ## New queries
 

config/identical-files.json

@@ -143,6 +143,11 @@
     "cpp/ql/src/semmle/code/cpp/ir/implementation/Opcode.qll",
     "csharp/ql/src/semmle/code/csharp/ir/implementation/Opcode.qll"
   ],
+  "IR SSASanity": [
+    "cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SSASanity.qll",
+    "cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/SSASanity.qll",
+    "csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/internal/SSASanity.qll"
+  ],
   "C++ IR InstructionImports": [
     "cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/InstructionImports.qll",
     "cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/InstructionImports.qll",

cpp/ql/src/Security/CWE/CWE-131/NoSpaceForZeroTerminator.ql

@@ -16,28 +16,28 @@
 
 import cpp
 import semmle.code.cpp.dataflow.DataFlow
-import semmle.code.cpp.models.implementations.Memcpy
+import semmle.code.cpp.models.interfaces.ArrayFunction
 
 class MallocCall extends FunctionCall {
   MallocCall() { this.getTarget().hasGlobalOrStdName("malloc") }
 
-  Expr getAllocatedSize() {
-    if this.getArgument(0) instanceof VariableAccess
-    then
-      exists(LocalScopeVariable v, ControlFlowNode def |
-        definitionUsePair(v, def, this.getArgument(0)) and
-        exprDefinition(v, def, result)
-      )
-    else result = this.getArgument(0)
-  }
+  Expr getAllocatedSize() { result = this.getArgument(0) }
 }
 
 predicate terminationProblem(MallocCall malloc, string msg) {
-  malloc.getAllocatedSize() instanceof StrlenCall and
-  not exists(FunctionCall fc, MemcpyFunction memcpy, int ix |
-    DataFlow::localExprFlow(malloc, fc.getArgument(ix)) and
-    fc.getTarget() = memcpy and
-    memcpy.hasArrayOutput(ix)
+  // malloc(strlen(...))
+  exists(StrlenCall strlen | DataFlow::localExprFlow(strlen, malloc.getAllocatedSize())) and
+  // flows into a null-terminated string function
+  exists(ArrayFunction af, FunctionCall fc, int arg |
+    DataFlow::localExprFlow(malloc, fc.getArgument(arg)) and
+    fc.getTarget() = af and
+    (
+      // null terminated string
+      af.hasArrayWithNullTerminator(arg)
+      or
+      // likely a null terminated string (such as `strcpy`, `strcat`)
+      af.hasArrayWithUnknownSize(arg)
+    )
   ) and
   msg = "This allocation does not include space to null-terminate the string."
 }

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

@@ -464,7 +464,7 @@ private predicate simpleParameterFlow(
 ) {
   throughFlowNodeCand(node, config) and
   p = node and
-  t = getErasedRepr(node.getType()) and
+  t = getErasedNodeType(node) and
   exists(ReturnNode ret, ReturnKind kind |
     returnNodeGetEnclosingCallable(ret) = p.getEnclosingCallable() and
     kind = ret.getKind() and
@@ -475,29 +475,29 @@ private predicate simpleParameterFlow(
   exists(Node mid |
     simpleParameterFlow(p, mid, t, config) and
     localFlowStep(mid, node, config) and
-    compatibleTypes(t, node.getType())
+    compatibleTypes(t, getErasedNodeType(node))
   )
   or
   throughFlowNodeCand(node, unbind(config)) and
   exists(Node mid |
     simpleParameterFlow(p, mid, _, config) and
     additionalLocalFlowStep(mid, node, config) and
-    t = getErasedRepr(node.getType())
+    t = getErasedNodeType(node)
   )
   or
   throughFlowNodeCand(node, unbind(config)) and
   exists(Node mid |
     simpleParameterFlow(p, mid, t, config) and
     localStoreReadStep(mid, node) and
-    compatibleTypes(t, node.getType())
+    compatibleTypes(t, getErasedNodeType(node))
   )
   or
   // value flow through a callable
   throughFlowNodeCand(node, unbind(config)) and
   exists(Node arg |
     simpleParameterFlow(p, arg, t, config) and
     argumentValueFlowsThrough(arg, node, _) and
-    compatibleTypes(t, node.getType())
+    compatibleTypes(t, getErasedNodeType(node))
   )
   or
   // flow through a callable
@@ -989,7 +989,9 @@ private class CastingNode extends Node {
  */
 private predicate flowCandFwd(Node node, boolean fromArg, AccessPathFront apf, Configuration config) {
   flowCandFwd0(node, fromArg, apf, config) and
-  if node instanceof CastingNode then compatibleTypes(node.getType(), apf.getType()) else any()
+  if node instanceof CastingNode
+  then compatibleTypes(getErasedNodeType(node), apf.getType())
+  else any()
 }
 
 /**
@@ -1010,7 +1012,7 @@ private class AccessPathFrontNilNode extends Node {
   }
 
   pragma[noinline]
-  private DataFlowType getErasedReprType() { result = getErasedRepr(this.getType()) }
+  private DataFlowType getErasedReprType() { result = getErasedNodeType(this) }
 
   /** Gets the `nil` path front for this node. */
   AccessPathFrontNil getApf() { result = TFrontNil(this.getErasedReprType()) }
@@ -1337,7 +1339,7 @@ private class AccessPathNilNode extends Node {
   AccessPathNilNode() { flowCand(this.(AccessPathFrontNilNode), _, _, _) }
 
   pragma[noinline]
-  private DataFlowType getErasedReprType() { result = getErasedRepr(this.getType()) }
+  private DataFlowType getErasedReprType() { result = getErasedNodeType(this) }
 
   /** Gets the `nil` path for this node. */
   AccessPathNil getAp() { result = TNil(this.getErasedReprType()) }
@@ -2076,7 +2078,7 @@ private module FlowExploration {
     TPartialPathNodeMk(Node node, CallContext cc, PartialAccessPath ap, Configuration config) {
       config.isSource(node) and
       cc instanceof CallContextAny and
-      ap = TPartialNil(getErasedRepr(node.getType())) and
+      ap = TPartialNil(getErasedNodeType(node)) and
       not fullBarrier(node, config) and
       exists(config.explorationLimit())
       or
@@ -2091,7 +2093,9 @@ private module FlowExploration {
     exists(PartialPathNode mid |
       partialPathStep(mid, node, cc, ap, config) and
       not fullBarrier(node, config) and
-      if node instanceof CastingNode then compatibleTypes(node.getType(), ap.getType()) else any()
+      if node instanceof CastingNode
+      then compatibleTypes(getErasedNodeType(node), ap.getType())
+      else any()
     )
   }
 
@@ -2194,7 +2198,7 @@ private module FlowExploration {
       additionalLocalFlowStep(mid.getNode(), node, config) and
       cc = mid.getCallContext() and
       mid.getAp() instanceof PartialAccessPathNil and
-      ap = TPartialNil(getErasedRepr(node.getType())) and
+      ap = TPartialNil(getErasedNodeType(node)) and
       config = mid.getConfiguration()
     )
     or
@@ -2206,7 +2210,7 @@ private module FlowExploration {
     additionalJumpStep(mid.getNode(), node, config) and
     cc instanceof CallContextAny and
     mid.getAp() instanceof PartialAccessPathNil and
-    ap = TPartialNil(getErasedRepr(node.getType())) and
+    ap = TPartialNil(getErasedNodeType(node)) and
     config = mid.getConfiguration()
     or
     partialPathStoreStep(mid, _, _, node, ap) and

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

@@ -464,7 +464,7 @@ private predicate simpleParameterFlow(
 ) {
   throughFlowNodeCand(node, config) and
   p = node and
-  t = getErasedRepr(node.getType()) and
+  t = getErasedNodeType(node) and
   exists(ReturnNode ret, ReturnKind kind |
     returnNodeGetEnclosingCallable(ret) = p.getEnclosingCallable() and
     kind = ret.getKind() and
@@ -475,29 +475,29 @@ private predicate simpleParameterFlow(
   exists(Node mid |
     simpleParameterFlow(p, mid, t, config) and
     localFlowStep(mid, node, config) and
-    compatibleTypes(t, node.getType())
+    compatibleTypes(t, getErasedNodeType(node))
   )
   or
   throughFlowNodeCand(node, unbind(config)) and
   exists(Node mid |
     simpleParameterFlow(p, mid, _, config) and
     additionalLocalFlowStep(mid, node, config) and
-    t = getErasedRepr(node.getType())
+    t = getErasedNodeType(node)
   )
   or
   throughFlowNodeCand(node, unbind(config)) and
   exists(Node mid |
     simpleParameterFlow(p, mid, t, config) and
     localStoreReadStep(mid, node) and
-    compatibleTypes(t, node.getType())
+    compatibleTypes(t, getErasedNodeType(node))
   )
   or
   // value flow through a callable
   throughFlowNodeCand(node, unbind(config)) and
   exists(Node arg |
     simpleParameterFlow(p, arg, t, config) and
     argumentValueFlowsThrough(arg, node, _) and
-    compatibleTypes(t, node.getType())
+    compatibleTypes(t, getErasedNodeType(node))
   )
   or
   // flow through a callable
@@ -989,7 +989,9 @@ private class CastingNode extends Node {
  */
 private predicate flowCandFwd(Node node, boolean fromArg, AccessPathFront apf, Configuration config) {
   flowCandFwd0(node, fromArg, apf, config) and
-  if node instanceof CastingNode then compatibleTypes(node.getType(), apf.getType()) else any()
+  if node instanceof CastingNode
+  then compatibleTypes(getErasedNodeType(node), apf.getType())
+  else any()
 }
 
 /**
@@ -1010,7 +1012,7 @@ private class AccessPathFrontNilNode extends Node {
   }
 
   pragma[noinline]
-  private DataFlowType getErasedReprType() { result = getErasedRepr(this.getType()) }
+  private DataFlowType getErasedReprType() { result = getErasedNodeType(this) }
 
   /** Gets the `nil` path front for this node. */
   AccessPathFrontNil getApf() { result = TFrontNil(this.getErasedReprType()) }
@@ -1337,7 +1339,7 @@ private class AccessPathNilNode extends Node {
   AccessPathNilNode() { flowCand(this.(AccessPathFrontNilNode), _, _, _) }
 
   pragma[noinline]
-  private DataFlowType getErasedReprType() { result = getErasedRepr(this.getType()) }
+  private DataFlowType getErasedReprType() { result = getErasedNodeType(this) }
 
   /** Gets the `nil` path for this node. */
   AccessPathNil getAp() { result = TNil(this.getErasedReprType()) }
@@ -2076,7 +2078,7 @@ private module FlowExploration {
     TPartialPathNodeMk(Node node, CallContext cc, PartialAccessPath ap, Configuration config) {
       config.isSource(node) and
       cc instanceof CallContextAny and
-      ap = TPartialNil(getErasedRepr(node.getType())) and
+      ap = TPartialNil(getErasedNodeType(node)) and
       not fullBarrier(node, config) and
       exists(config.explorationLimit())
       or
@@ -2091,7 +2093,9 @@ private module FlowExploration {
     exists(PartialPathNode mid |
       partialPathStep(mid, node, cc, ap, config) and
       not fullBarrier(node, config) and
-      if node instanceof CastingNode then compatibleTypes(node.getType(), ap.getType()) else any()
+      if node instanceof CastingNode
+      then compatibleTypes(getErasedNodeType(node), ap.getType())
+      else any()
     )
   }
 
@@ -2194,7 +2198,7 @@ private module FlowExploration {
       additionalLocalFlowStep(mid.getNode(), node, config) and
       cc = mid.getCallContext() and
       mid.getAp() instanceof PartialAccessPathNil and
-      ap = TPartialNil(getErasedRepr(node.getType())) and
+      ap = TPartialNil(getErasedNodeType(node)) and
       config = mid.getConfiguration()
     )
     or
@@ -2206,7 +2210,7 @@ private module FlowExploration {
     additionalJumpStep(mid.getNode(), node, config) and
     cc instanceof CallContextAny and
     mid.getAp() instanceof PartialAccessPathNil and
-    ap = TPartialNil(getErasedRepr(node.getType())) and
+    ap = TPartialNil(getErasedNodeType(node)) and
     config = mid.getConfiguration()
     or
     partialPathStoreStep(mid, _, _, node, ap) and