Merge branch 'master' into get-an-assigned-value-join-order

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

@@ -2293,12 +2293,13 @@ private class PathNodeSink extends PathNodeImpl, TPathNodeSink {
  * a callable is recorded by `cc`.
  */
 private predicate pathStep(PathNodeMid mid, Node node, CallContext cc, SummaryCtx sc, AccessPath ap) {
-  exists(
-    AccessPath ap0, Node midnode, Configuration conf, DataFlowCallable enclosing,
-    LocalCallContext localCC
-  |
-    pathIntoLocalStep(mid, midnode, cc, enclosing, sc, ap0, conf) and
-    localCC = getLocalCallContext(cc, enclosing)
+  exists(AccessPath ap0, Node midnode, Configuration conf, LocalCallContext localCC |
+    midnode = mid.getNode() and
+    conf = mid.getConfiguration() and
+    cc = mid.getCallContext() and
+    sc = mid.getSummaryCtx() and
+    localCC = getLocalCallContext(cc, midnode.getEnclosingCallable()) and
+    ap0 = mid.getAp()
   |
     localFlowBigStep(midnode, node, true, _, conf, localCC) and
     ap = ap0
@@ -2331,20 +2332,6 @@ private predicate pathStep(PathNodeMid mid, Node node, CallContext cc, SummaryCt
   pathThroughCallable(mid, node, cc, ap) and sc = mid.getSummaryCtx()
 }
 
-pragma[nomagic]
-private predicate pathIntoLocalStep(
-  PathNodeMid mid, Node midnode, CallContext cc, DataFlowCallable enclosing, SummaryCtx sc,
-  AccessPath ap0, Configuration conf
-) {
-  midnode = mid.getNode() and
-  cc = mid.getCallContext() and
-  conf = mid.getConfiguration() and
-  localFlowBigStep(midnode, _, _, _, conf, _) and
-  enclosing = midnode.getEnclosingCallable() and
-  sc = mid.getSummaryCtx() and
-  ap0 = mid.getAp()
-}
-
 pragma[nomagic]
 private predicate readCand(Node node1, Content f, Node node2, Configuration config) {
   readDirect(node1, f, node2) and

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

@@ -2293,12 +2293,13 @@ private class PathNodeSink extends PathNodeImpl, TPathNodeSink {
  * a callable is recorded by `cc`.
  */
 private predicate pathStep(PathNodeMid mid, Node node, CallContext cc, SummaryCtx sc, AccessPath ap) {
-  exists(
-    AccessPath ap0, Node midnode, Configuration conf, DataFlowCallable enclosing,
-    LocalCallContext localCC
-  |
-    pathIntoLocalStep(mid, midnode, cc, enclosing, sc, ap0, conf) and
-    localCC = getLocalCallContext(cc, enclosing)
+  exists(AccessPath ap0, Node midnode, Configuration conf, LocalCallContext localCC |
+    midnode = mid.getNode() and
+    conf = mid.getConfiguration() and
+    cc = mid.getCallContext() and
+    sc = mid.getSummaryCtx() and
+    localCC = getLocalCallContext(cc, midnode.getEnclosingCallable()) and
+    ap0 = mid.getAp()
   |
     localFlowBigStep(midnode, node, true, _, conf, localCC) and
     ap = ap0
@@ -2331,20 +2332,6 @@ private predicate pathStep(PathNodeMid mid, Node node, CallContext cc, SummaryCt
   pathThroughCallable(mid, node, cc, ap) and sc = mid.getSummaryCtx()
 }
 
-pragma[nomagic]
-private predicate pathIntoLocalStep(
-  PathNodeMid mid, Node midnode, CallContext cc, DataFlowCallable enclosing, SummaryCtx sc,
-  AccessPath ap0, Configuration conf
-) {
-  midnode = mid.getNode() and
-  cc = mid.getCallContext() and
-  conf = mid.getConfiguration() and
-  localFlowBigStep(midnode, _, _, _, conf, _) and
-  enclosing = midnode.getEnclosingCallable() and
-  sc = mid.getSummaryCtx() and
-  ap0 = mid.getAp()
-}
-
 pragma[nomagic]
 private predicate readCand(Node node1, Content f, Node node2, Configuration config) {
   readDirect(node1, f, node2) and

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

@@ -2293,12 +2293,13 @@ private class PathNodeSink extends PathNodeImpl, TPathNodeSink {
  * a callable is recorded by `cc`.
  */
 private predicate pathStep(PathNodeMid mid, Node node, CallContext cc, SummaryCtx sc, AccessPath ap) {
-  exists(
-    AccessPath ap0, Node midnode, Configuration conf, DataFlowCallable enclosing,
-    LocalCallContext localCC
-  |
-    pathIntoLocalStep(mid, midnode, cc, enclosing, sc, ap0, conf) and
-    localCC = getLocalCallContext(cc, enclosing)
+  exists(AccessPath ap0, Node midnode, Configuration conf, LocalCallContext localCC |
+    midnode = mid.getNode() and
+    conf = mid.getConfiguration() and
+    cc = mid.getCallContext() and
+    sc = mid.getSummaryCtx() and
+    localCC = getLocalCallContext(cc, midnode.getEnclosingCallable()) and
+    ap0 = mid.getAp()
   |
     localFlowBigStep(midnode, node, true, _, conf, localCC) and
     ap = ap0
@@ -2331,20 +2332,6 @@ private predicate pathStep(PathNodeMid mid, Node node, CallContext cc, SummaryCt
   pathThroughCallable(mid, node, cc, ap) and sc = mid.getSummaryCtx()
 }
 
-pragma[nomagic]
-private predicate pathIntoLocalStep(
-  PathNodeMid mid, Node midnode, CallContext cc, DataFlowCallable enclosing, SummaryCtx sc,
-  AccessPath ap0, Configuration conf
-) {
-  midnode = mid.getNode() and
-  cc = mid.getCallContext() and
-  conf = mid.getConfiguration() and
-  localFlowBigStep(midnode, _, _, _, conf, _) and
-  enclosing = midnode.getEnclosingCallable() and
-  sc = mid.getSummaryCtx() and
-  ap0 = mid.getAp()
-}
-
 pragma[nomagic]
 private predicate readCand(Node node1, Content f, Node node2, Configuration config) {
   readDirect(node1, f, node2) and

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

@@ -2293,12 +2293,13 @@ private class PathNodeSink extends PathNodeImpl, TPathNodeSink {
  * a callable is recorded by `cc`.
  */
 private predicate pathStep(PathNodeMid mid, Node node, CallContext cc, SummaryCtx sc, AccessPath ap) {
-  exists(
-    AccessPath ap0, Node midnode, Configuration conf, DataFlowCallable enclosing,
-    LocalCallContext localCC
-  |
-    pathIntoLocalStep(mid, midnode, cc, enclosing, sc, ap0, conf) and
-    localCC = getLocalCallContext(cc, enclosing)
+  exists(AccessPath ap0, Node midnode, Configuration conf, LocalCallContext localCC |
+    midnode = mid.getNode() and
+    conf = mid.getConfiguration() and
+    cc = mid.getCallContext() and
+    sc = mid.getSummaryCtx() and
+    localCC = getLocalCallContext(cc, midnode.getEnclosingCallable()) and
+    ap0 = mid.getAp()
   |
     localFlowBigStep(midnode, node, true, _, conf, localCC) and
     ap = ap0
@@ -2331,20 +2332,6 @@ private predicate pathStep(PathNodeMid mid, Node node, CallContext cc, SummaryCt
   pathThroughCallable(mid, node, cc, ap) and sc = mid.getSummaryCtx()
 }
 
-pragma[nomagic]
-private predicate pathIntoLocalStep(
-  PathNodeMid mid, Node midnode, CallContext cc, DataFlowCallable enclosing, SummaryCtx sc,
-  AccessPath ap0, Configuration conf
-) {
-  midnode = mid.getNode() and
-  cc = mid.getCallContext() and
-  conf = mid.getConfiguration() and
-  localFlowBigStep(midnode, _, _, _, conf, _) and
-  enclosing = midnode.getEnclosingCallable() and
-  sc = mid.getSummaryCtx() and
-  ap0 = mid.getAp()
-}
-
 pragma[nomagic]
 private predicate readCand(Node node1, Content f, Node node2, Configuration config) {
   readDirect(node1, f, node2) and

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

@@ -2293,12 +2293,13 @@ private class PathNodeSink extends PathNodeImpl, TPathNodeSink {
  * a callable is recorded by `cc`.
  */
 private predicate pathStep(PathNodeMid mid, Node node, CallContext cc, SummaryCtx sc, AccessPath ap) {
-  exists(
-    AccessPath ap0, Node midnode, Configuration conf, DataFlowCallable enclosing,
-    LocalCallContext localCC
-  |
-    pathIntoLocalStep(mid, midnode, cc, enclosing, sc, ap0, conf) and
-    localCC = getLocalCallContext(cc, enclosing)
+  exists(AccessPath ap0, Node midnode, Configuration conf, LocalCallContext localCC |
+    midnode = mid.getNode() and
+    conf = mid.getConfiguration() and
+    cc = mid.getCallContext() and
+    sc = mid.getSummaryCtx() and
+    localCC = getLocalCallContext(cc, midnode.getEnclosingCallable()) and
+    ap0 = mid.getAp()
   |
     localFlowBigStep(midnode, node, true, _, conf, localCC) and
     ap = ap0
@@ -2331,20 +2332,6 @@ private predicate pathStep(PathNodeMid mid, Node node, CallContext cc, SummaryCt
   pathThroughCallable(mid, node, cc, ap) and sc = mid.getSummaryCtx()
 }
 
-pragma[nomagic]
-private predicate pathIntoLocalStep(
-  PathNodeMid mid, Node midnode, CallContext cc, DataFlowCallable enclosing, SummaryCtx sc,
-  AccessPath ap0, Configuration conf
-) {
-  midnode = mid.getNode() and
-  cc = mid.getCallContext() and
-  conf = mid.getConfiguration() and
-  localFlowBigStep(midnode, _, _, _, conf, _) and
-  enclosing = midnode.getEnclosingCallable() and
-  sc = mid.getSummaryCtx() and
-  ap0 = mid.getAp()
-}
-
 pragma[nomagic]
 private predicate readCand(Node node1, Content f, Node node2, Configuration config) {
   readDirect(node1, f, node2) and

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

@@ -43,7 +43,7 @@ class Node extends TNode {
   /**
    * INTERNAL: Do not use. Alternative name for `getFunction`.
    */
-  Function getEnclosingCallable() { result = this.getFunction() }
+  final Function getEnclosingCallable() { result = unique(Function f | f = this.getFunction() | f) }
 
   /** Gets the type of this node. */
   Type getType() { none() } // overridden in subclasses

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

@@ -2293,12 +2293,13 @@ private class PathNodeSink extends PathNodeImpl, TPathNodeSink {
  * a callable is recorded by `cc`.
  */
 private predicate pathStep(PathNodeMid mid, Node node, CallContext cc, SummaryCtx sc, AccessPath ap) {
-  exists(
-    AccessPath ap0, Node midnode, Configuration conf, DataFlowCallable enclosing,
-    LocalCallContext localCC
-  |
-    pathIntoLocalStep(mid, midnode, cc, enclosing, sc, ap0, conf) and
-    localCC = getLocalCallContext(cc, enclosing)
+  exists(AccessPath ap0, Node midnode, Configuration conf, LocalCallContext localCC |
+    midnode = mid.getNode() and
+    conf = mid.getConfiguration() and
+    cc = mid.getCallContext() and
+    sc = mid.getSummaryCtx() and
+    localCC = getLocalCallContext(cc, midnode.getEnclosingCallable()) and
+    ap0 = mid.getAp()
   |
     localFlowBigStep(midnode, node, true, _, conf, localCC) and
     ap = ap0
@@ -2331,20 +2332,6 @@ private predicate pathStep(PathNodeMid mid, Node node, CallContext cc, SummaryCt
   pathThroughCallable(mid, node, cc, ap) and sc = mid.getSummaryCtx()
 }
 
-pragma[nomagic]
-private predicate pathIntoLocalStep(
-  PathNodeMid mid, Node midnode, CallContext cc, DataFlowCallable enclosing, SummaryCtx sc,
-  AccessPath ap0, Configuration conf
-) {
-  midnode = mid.getNode() and
-  cc = mid.getCallContext() and
-  conf = mid.getConfiguration() and
-  localFlowBigStep(midnode, _, _, _, conf, _) and
-  enclosing = midnode.getEnclosingCallable() and
-  sc = mid.getSummaryCtx() and
-  ap0 = mid.getAp()
-}
-
 pragma[nomagic]
 private predicate readCand(Node node1, Content f, Node node2, Configuration config) {
   readDirect(node1, f, node2) and

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

@@ -2293,12 +2293,13 @@ private class PathNodeSink extends PathNodeImpl, TPathNodeSink {
  * a callable is recorded by `cc`.
  */
 private predicate pathStep(PathNodeMid mid, Node node, CallContext cc, SummaryCtx sc, AccessPath ap) {
-  exists(
-    AccessPath ap0, Node midnode, Configuration conf, DataFlowCallable enclosing,
-    LocalCallContext localCC
-  |
-    pathIntoLocalStep(mid, midnode, cc, enclosing, sc, ap0, conf) and
-    localCC = getLocalCallContext(cc, enclosing)
+  exists(AccessPath ap0, Node midnode, Configuration conf, LocalCallContext localCC |
+    midnode = mid.getNode() and
+    conf = mid.getConfiguration() and
+    cc = mid.getCallContext() and
+    sc = mid.getSummaryCtx() and
+    localCC = getLocalCallContext(cc, midnode.getEnclosingCallable()) and
+    ap0 = mid.getAp()
   |
     localFlowBigStep(midnode, node, true, _, conf, localCC) and
     ap = ap0
@@ -2331,20 +2332,6 @@ private predicate pathStep(PathNodeMid mid, Node node, CallContext cc, SummaryCt
   pathThroughCallable(mid, node, cc, ap) and sc = mid.getSummaryCtx()
 }
 
-pragma[nomagic]
-private predicate pathIntoLocalStep(
-  PathNodeMid mid, Node midnode, CallContext cc, DataFlowCallable enclosing, SummaryCtx sc,
-  AccessPath ap0, Configuration conf
-) {
-  midnode = mid.getNode() and
-  cc = mid.getCallContext() and
-  conf = mid.getConfiguration() and
-  localFlowBigStep(midnode, _, _, _, conf, _) and
-  enclosing = midnode.getEnclosingCallable() and
-  sc = mid.getSummaryCtx() and
-  ap0 = mid.getAp()
-}
-
 pragma[nomagic]
 private predicate readCand(Node node1, Content f, Node node2, Configuration config) {
   readDirect(node1, f, node2) and

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

@@ -2293,12 +2293,13 @@ private class PathNodeSink extends PathNodeImpl, TPathNodeSink {
  * a callable is recorded by `cc`.
  */
 private predicate pathStep(PathNodeMid mid, Node node, CallContext cc, SummaryCtx sc, AccessPath ap) {
-  exists(
-    AccessPath ap0, Node midnode, Configuration conf, DataFlowCallable enclosing,
-    LocalCallContext localCC
-  |
-    pathIntoLocalStep(mid, midnode, cc, enclosing, sc, ap0, conf) and
-    localCC = getLocalCallContext(cc, enclosing)
+  exists(AccessPath ap0, Node midnode, Configuration conf, LocalCallContext localCC |
+    midnode = mid.getNode() and
+    conf = mid.getConfiguration() and
+    cc = mid.getCallContext() and
+    sc = mid.getSummaryCtx() and
+    localCC = getLocalCallContext(cc, midnode.getEnclosingCallable()) and
+    ap0 = mid.getAp()
   |
     localFlowBigStep(midnode, node, true, _, conf, localCC) and
     ap = ap0
@@ -2331,20 +2332,6 @@ private predicate pathStep(PathNodeMid mid, Node node, CallContext cc, SummaryCt
   pathThroughCallable(mid, node, cc, ap) and sc = mid.getSummaryCtx()
 }
 
-pragma[nomagic]
-private predicate pathIntoLocalStep(
-  PathNodeMid mid, Node midnode, CallContext cc, DataFlowCallable enclosing, SummaryCtx sc,
-  AccessPath ap0, Configuration conf
-) {
-  midnode = mid.getNode() and
-  cc = mid.getCallContext() and
-  conf = mid.getConfiguration() and
-  localFlowBigStep(midnode, _, _, _, conf, _) and
-  enclosing = midnode.getEnclosingCallable() and
-  sc = mid.getSummaryCtx() and
-  ap0 = mid.getAp()
-}
-
 pragma[nomagic]
 private predicate readCand(Node node1, Content f, Node node2, Configuration config) {
   readDirect(node1, f, node2) and

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

@@ -2293,12 +2293,13 @@ private class PathNodeSink extends PathNodeImpl, TPathNodeSink {
  * a callable is recorded by `cc`.
  */
 private predicate pathStep(PathNodeMid mid, Node node, CallContext cc, SummaryCtx sc, AccessPath ap) {
-  exists(
-    AccessPath ap0, Node midnode, Configuration conf, DataFlowCallable enclosing,
-    LocalCallContext localCC
-  |
-    pathIntoLocalStep(mid, midnode, cc, enclosing, sc, ap0, conf) and
-    localCC = getLocalCallContext(cc, enclosing)
+  exists(AccessPath ap0, Node midnode, Configuration conf, LocalCallContext localCC |
+    midnode = mid.getNode() and
+    conf = mid.getConfiguration() and
+    cc = mid.getCallContext() and
+    sc = mid.getSummaryCtx() and
+    localCC = getLocalCallContext(cc, midnode.getEnclosingCallable()) and
+    ap0 = mid.getAp()
   |
     localFlowBigStep(midnode, node, true, _, conf, localCC) and
     ap = ap0
@@ -2331,20 +2332,6 @@ private predicate pathStep(PathNodeMid mid, Node node, CallContext cc, SummaryCt
   pathThroughCallable(mid, node, cc, ap) and sc = mid.getSummaryCtx()
 }
 
-pragma[nomagic]
-private predicate pathIntoLocalStep(
-  PathNodeMid mid, Node midnode, CallContext cc, DataFlowCallable enclosing, SummaryCtx sc,
-  AccessPath ap0, Configuration conf
-) {
-  midnode = mid.getNode() and
-  cc = mid.getCallContext() and
-  conf = mid.getConfiguration() and
-  localFlowBigStep(midnode, _, _, _, conf, _) and
-  enclosing = midnode.getEnclosingCallable() and
-  sc = mid.getSummaryCtx() and
-  ap0 = mid.getAp()
-}
-
 pragma[nomagic]
 private predicate readCand(Node node1, Content f, Node node2, Configuration config) {
   readDirect(node1, f, node2) and

cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/ModelUtil.qll

@@ -0,0 +1,47 @@
+/**
+ * Provides predicates for mapping the `FunctionInput` and `FunctionOutput`
+ * classes used in function models to the corresponding instructions.
+ */
+
+private import semmle.code.cpp.ir.IR
+private import semmle.code.cpp.ir.dataflow.DataFlow
+
+/**
+ * Gets the instruction that goes into `input` for `call`.
+ */
+Instruction callInput(CallInstruction call, FunctionInput input) {
+  // A positional argument
+  exists(int index |
+    result = call.getPositionalArgument(index) and
+    input.isParameter(index)
+  )
+  or
+  // A value pointed to by a positional argument
+  exists(ReadSideEffectInstruction read |
+    result = read and
+    read.getPrimaryInstruction() = call and
+    input.isParameterDeref(read.getIndex())
+  )
+  or
+  // The qualifier pointer
+  result = call.getThisArgument() and
+  input.isQualifierAddress()
+  //TODO: qualifier deref
+}
+
+/**
+ * Gets the instruction that holds the `output` for `call`.
+ */
+Instruction callOutput(CallInstruction call, FunctionOutput output) {
+  // The return value
+  result = call and
+  output.isReturnValue()
+  or
+  // The side effect of a call on the value pointed to by a positional argument
+  exists(WriteSideEffectInstruction effect |
+    result = effect and
+    effect.getPrimaryInstruction() = call and
+    output.isParameterDeref(effect.getIndex())
+  )
+  // TODO: qualifiers, return value dereference
+}

cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/TaintTrackingUtil.qll

@@ -1,5 +1,8 @@
 private import semmle.code.cpp.ir.IR
 private import semmle.code.cpp.ir.dataflow.DataFlow
+private import ModelUtil
+private import semmle.code.cpp.models.interfaces.DataFlow
+private import semmle.code.cpp.models.interfaces.SideEffect
 
 /**
  * Holds if taint propagates from `nodeFrom` to `nodeTo` in exactly one local
@@ -45,6 +48,25 @@ private predicate localInstructionTaintStep(Instruction nodeFrom, Instruction no
   )
   or
   nodeTo.(LoadInstruction).getSourceAddress() = nodeFrom
+  or
+  modeledInstructionTaintStep(nodeFrom, nodeTo)
+  or
+  // Flow through partial reads of arrays and unions
+  nodeTo.(LoadInstruction).getSourceValueOperand().getAnyDef() = nodeFrom and
+  not nodeFrom.isResultConflated() and
+  (
+    nodeFrom.getResultType() instanceof ArrayType or
+    nodeFrom.getResultType() instanceof Union
+  )
+  or
+  // Flow from an element to an array or union that contains it.
+  nodeTo.(ChiInstruction).getPartial() = nodeFrom and
+  not nodeTo.isResultConflated() and
+  exists(Type t | nodeTo.getResultLanguageType().hasType(t, false) |
+    t instanceof Union
+    or
+    t instanceof ArrayType
+  )
 }
 
 /**
@@ -82,3 +104,34 @@ predicate defaultAdditionalTaintStep(DataFlow::Node src, DataFlow::Node sink) {
  * but not in local taint.
  */
 predicate defaultTaintBarrier(DataFlow::Node node) { none() }
+
+/**
+ * Holds if taint can flow from `instrIn` to `instrOut` through a call to a
+ * modeled function.
+ */
+predicate modeledInstructionTaintStep(Instruction instrIn, Instruction instrOut) {
+  exists(CallInstruction call, TaintFunction func, FunctionInput modelIn, FunctionOutput modelOut |
+    instrIn = callInput(call, modelIn) and
+    instrOut = callOutput(call, modelOut) and
+    call.getStaticCallTarget() = func and
+    func.hasTaintFlow(modelIn, modelOut)
+  )
+  or
+  // Taint flow from one argument to another and data flow from an argument to a
+  // return value. This happens in functions like `strcat` and `memcpy`. We
+  // could model this flow in two separate steps, but that would add reverse
+  // flow from the write side-effect to the call instruction, which may not be
+  // desirable.
+  exists(
+    CallInstruction call, Function func, FunctionInput modelIn, OutParameterDeref modelMidOut,
+    int indexMid, InParameter modelMidIn, OutReturnValue modelOut
+  |
+    instrIn = callInput(call, modelIn) and
+    instrOut = callOutput(call, modelOut) and
+    call.getStaticCallTarget() = func and
+    func.(TaintFunction).hasTaintFlow(modelIn, modelMidOut) and
+    func.(DataFlowFunction).hasDataFlow(modelMidIn, modelOut) and
+    modelMidOut.isParameterDeref(indexMid) and
+    modelMidIn.isParameter(indexMid)
+  )
+}

cpp/ql/src/semmle/code/cpp/models/implementations/Allocation.qll

@@ -89,6 +89,18 @@ class MallocAllocationFunction extends AllocationFunction {
         or
         // kmem_zalloc(size, flags)
         name = "kmem_zalloc" and sizeArg = 0
+        or
+        // CRYPTO_malloc(size_t num, const char *file, int line)
+        name = "CRYPTO_malloc" and sizeArg = 0
+        or
+        // CRYPTO_zalloc(size_t num, const char *file, int line)
+        name = "CRYPTO_zalloc" and sizeArg = 0
+        or
+        // CRYPTO_secure_malloc(size_t num, const char *file, int line)
+        name = "CRYPTO_secure_malloc" and sizeArg = 0
+        or
+        // CRYPTO_secure_zalloc(size_t num, const char *file, int line)
+        name = "CRYPTO_secure_zalloc" and sizeArg = 0
       )
     )
   }
@@ -169,6 +181,9 @@ class ReallocAllocationFunction extends AllocationFunction {
         or
         // CoTaskMemRealloc(ptr, size)
         name = "CoTaskMemRealloc" and sizeArg = 1 and reallocArg = 0
+        or
+        // CRYPTO_realloc(void *addr, size_t num, const char *file, int line);
+        name = "CRYPTO_realloc" and sizeArg = 1 and reallocArg = 0
       )
     )
   }
@@ -255,6 +270,36 @@ class OperatorNewAllocationFunction extends AllocationFunction {
   }
 }
 
+/**
+ * The predicate analyzes a `sizeExpr`, which is an argument to an allocation
+ * function like malloc, and tries to split it into an expression `lengthExpr`
+ * that describes the length of the allocated array, and the size of the allocated
+ * element type `sizeof`.
+ * If this is not possible, the allocation is considered to be of size 1 and of
+ * length `sizeExpr`.
+ */
+private predicate deconstructSizeExpr(Expr sizeExpr, Expr lengthExpr, int sizeof) {
+  if
+    sizeExpr instanceof MulExpr and
+    exists(SizeofOperator sizeofOp, Expr lengthOp |
+      sizeofOp = sizeExpr.(MulExpr).getAnOperand() and
+      lengthOp = sizeExpr.(MulExpr).getAnOperand() and
+      not lengthOp instanceof SizeofOperator and
+      exists(sizeofOp.getValue().toInt())
+    )
+  then
+    exists(SizeofOperator sizeofOp |
+      sizeofOp = sizeExpr.(MulExpr).getAnOperand() and
+      lengthExpr = sizeExpr.(MulExpr).getAnOperand() and
+      not lengthExpr instanceof SizeofOperator and
+      sizeof = sizeofOp.getValue().toInt()
+    )
+  else (
+    lengthExpr = sizeExpr and
+    sizeof = 1
+  )
+}
+
 /**
  * An allocation expression that is a function call, such as call to `malloc`.
  */
@@ -272,7 +317,17 @@ class CallAllocationExpr extends AllocationExpr, FunctionCall {
     not exists(NewOrNewArrayExpr new | new.getAllocatorCall() = this)
   }
 
-  override Expr getSizeExpr() { result = getArgument(target.getSizeArg()) }
+  override Expr getSizeExpr() {
+    exists(Expr sizeExpr | sizeExpr = getArgument(target.getSizeArg()) |
+      if exists(target.getSizeMult())
+      then result = sizeExpr
+      else
+        exists(Expr lengthExpr |
+          deconstructSizeExpr(sizeExpr, lengthExpr, _) and
+          result = lengthExpr
+        )
+    )
+  }
 
   override int getSizeMult() {
     // malloc with multiplier argument that is a constant
@@ -280,7 +335,7 @@ class CallAllocationExpr extends AllocationExpr, FunctionCall {
     or
     // malloc with no multiplier argument
     not exists(target.getSizeMult()) and
-    result = 1
+    deconstructSizeExpr(getArgument(target.getSizeArg()), _, result)
   }
 
   override int getSizeBytes() { result = getSizeExpr().getValue().toInt() * getSizeMult() }

cpp/ql/src/semmle/code/cpp/models/implementations/Deallocation.qll

@@ -19,6 +19,10 @@ class StandardDeallocationFunction extends DeallocationFunction {
         name = "free" and freedArg = 0
         or
         name = "realloc" and freedArg = 0
+        or
+        name = "CRYPTO_free" and freedArg = 0
+        or
+        name = "CRYPTO_secure_free" and freedArg = 0
       )
       or
       hasGlobalOrStdName(name) and

cpp/ql/test/library-tests/allocators/allocators.cpp

@@ -149,3 +149,15 @@ void directOperatorCall() {
 	ptr = operator new(sizeof(int));
 	operator delete(ptr);
 }
+
+void *malloc(size_t);
+
+void testMalloc(size_t count) {
+  malloc(5);
+  malloc(5 * sizeof(int));
+  malloc(count);
+  malloc(count * sizeof(int));
+  malloc(count * sizeof(int) + 1);
+  malloc(((int) count) * sizeof(void *));
+  malloc(sizeof(void*) * sizeof(int));
+}

cpp/ql/test/library-tests/allocators/allocators.expected

@@ -55,6 +55,7 @@ allocationFunctions
 | allocators.cpp:122:7:122:20 | operator new[] | getPlacementArgument = 1, getSizeArg = 0 |
 | allocators.cpp:123:7:123:18 | operator new | getSizeArg = 0, requiresDealloc |
 | allocators.cpp:124:7:124:20 | operator new[] | getSizeArg = 0, requiresDealloc |
+| allocators.cpp:153:7:153:12 | malloc | getSizeArg = 0, requiresDealloc |
 | file://:0:0:0:0 | operator new | getSizeArg = 0, requiresDealloc |
 | file://:0:0:0:0 | operator new | getSizeArg = 0, requiresDealloc |
 | file://:0:0:0:0 | operator new[] | getSizeArg = 0, requiresDealloc |
@@ -84,6 +85,13 @@ allocationExprs
 | allocators.cpp:143:13:143:28 | new[] | getSizeBytes = 400, requiresDealloc |
 | allocators.cpp:144:13:144:31 | new[] | getSizeExpr = x, getSizeMult = 900, requiresDealloc |
 | allocators.cpp:149:8:149:19 | call to operator new | getSizeBytes = 4, getSizeExpr = sizeof(int), getSizeMult = 1, requiresDealloc |
+| allocators.cpp:156:3:156:8 | call to malloc | getSizeBytes = 5, getSizeExpr = 5, getSizeMult = 1, requiresDealloc |
+| allocators.cpp:157:3:157:8 | call to malloc | getSizeBytes = 20, getSizeExpr = 5, getSizeMult = 4, requiresDealloc |
+| allocators.cpp:158:3:158:8 | call to malloc | getSizeExpr = count, getSizeMult = 1, requiresDealloc |
+| allocators.cpp:159:3:159:8 | call to malloc | getSizeExpr = count, getSizeMult = 4, requiresDealloc |
+| allocators.cpp:160:3:160:8 | call to malloc | getSizeExpr = ... + ..., getSizeMult = 1, requiresDealloc |
+| allocators.cpp:161:3:161:8 | call to malloc | getSizeExpr = count, getSizeMult = 8, requiresDealloc |
+| allocators.cpp:162:3:162:8 | call to malloc | getSizeBytes = 32, getSizeExpr = ... * ..., getSizeMult = 1, requiresDealloc |
 deallocationFunctions
 | allocators.cpp:11:6:11:20 | operator delete | getFreedArg = 0 |
 | allocators.cpp:12:6:12:22 | operator delete[] | getFreedArg = 0 |

cpp/ql/test/library-tests/dataflow/taint-tests/test_diff.expected

@@ -22,6 +22,9 @@
 | taint.cpp:93:11:93:11 | taint.cpp:71:22:71:27 | AST only |
 | taint.cpp:94:11:94:11 | taint.cpp:72:7:72:12 | AST only |
 | taint.cpp:109:7:109:13 | taint.cpp:105:12:105:17 | IR only |
+| taint.cpp:110:7:110:13 | taint.cpp:105:12:105:17 | IR only |
+| taint.cpp:111:7:111:13 | taint.cpp:106:12:106:17 | IR only |
+| taint.cpp:112:7:112:13 | taint.cpp:106:12:106:17 | IR only |
 | taint.cpp:130:7:130:9 | taint.cpp:127:8:127:13 | IR only |
 | taint.cpp:137:7:137:9 | taint.cpp:120:11:120:16 | AST only |
 | taint.cpp:173:8:173:13 | taint.cpp:164:19:164:24 | AST only |

cpp/ql/test/library-tests/dataflow/taint-tests/test_ir.expected

@@ -4,6 +4,9 @@
 | taint.cpp:16:8:16:14 | source1 | taint.cpp:12:22:12:27 | call to source |
 | taint.cpp:17:8:17:16 | ++ ... | taint.cpp:12:22:12:27 | call to source |
 | taint.cpp:109:7:109:13 | access to array | taint.cpp:105:12:105:17 | call to source |
+| taint.cpp:110:7:110:13 | access to array | taint.cpp:105:12:105:17 | call to source |
+| taint.cpp:111:7:111:13 | access to array | taint.cpp:106:12:106:17 | call to source |
+| taint.cpp:112:7:112:13 | access to array | taint.cpp:106:12:106:17 | call to source |
 | taint.cpp:129:7:129:9 | * ... | taint.cpp:120:11:120:16 | call to source |
 | taint.cpp:130:7:130:9 | * ... | taint.cpp:127:8:127:13 | call to source |
 | taint.cpp:134:7:134:9 | * ... | taint.cpp:120:11:120:16 | call to source |

cpp/ql/test/query-tests/Critical/OverflowCalculated/NoSpaceForZeroTerminator.expected

@@ -1,7 +1,9 @@
 | tests1.cpp:26:21:26:26 | call to malloc | This allocation does not include space to null-terminate the string. |
+| tests1.cpp:36:21:36:26 | call to malloc | This allocation does not include space to null-terminate the string. |
 | tests1.cpp:56:21:56:27 | call to realloc | This allocation does not include space to null-terminate the string. |
 | tests1.cpp:67:21:67:26 | call to malloc | This allocation does not include space to null-terminate the string. |
 | tests1.cpp:89:25:89:30 | call to malloc | This allocation does not include space to null-terminate the string. |
+| tests1.cpp:109:25:109:30 | call to malloc | This allocation does not include space to null-terminate the string. |
 | tests3.cpp:25:21:25:31 | call to malloc | This allocation does not include space to null-terminate the string. |
 | tests3.cpp:30:21:30:31 | call to malloc | This allocation does not include space to null-terminate the string. |
 | tests3.cpp:53:17:53:44 | new[] | This allocation does not include space to null-terminate the string. |

cpp/ql/test/query-tests/Critical/OverflowCalculated/OverflowCalculated.expected

@@ -1 +1,2 @@
 | tests2.cpp:34:4:34:9 | call to strcat | This buffer only contains enough room for 'str1' (copied on line 33) |
+| tests2.cpp:52:4:52:9 | call to strcat | This buffer only contains enough room for 'str1' (copied on line 51) |

cpp/ql/test/query-tests/Critical/OverflowCalculated/tests1.cpp

@@ -33,7 +33,7 @@ void tests1(int case_num)
 			break;
 
 		case 3:
-			buffer = (char *)malloc(strlen(str) * sizeof(char)); // BAD [NOT DETECTED]
+			buffer = (char *)malloc(strlen(str) * sizeof(char)); // BAD
 			strcpy(buffer, str);
 			break;
 
@@ -106,7 +106,7 @@ void tests1(int case_num)
 			break;
 
 		case 105:
-			wbuffer = (wchar_t *)malloc(wcslen(wstr) * sizeof(wchar_t)); // BAD [NOT DETECTED]
+			wbuffer = (wchar_t *)malloc(wcslen(wstr) * sizeof(wchar_t)); // BAD
 			wcscpy(wbuffer, wstr);
 			break;
 

cpp/ql/test/query-tests/Critical/OverflowCalculated/tests2.cpp

@@ -47,7 +47,7 @@ void tests2(int case_num)
 			break;
 
 		case 4:
-			buffer = (char *)malloc((strlen(str1) + 1) * sizeof(char)); // BAD [NOT DETECTED]
+			buffer = (char *)malloc((strlen(str1) + 1) * sizeof(char)); // BAD
 			strcpy(buffer, str1);
 			strcat(buffer, str2);
 			break;

cpp/ql/test/query-tests/Security/CWE/CWE-131/semmle/NoSpaceForZeroTerminator/NoSpaceForZeroTerminator.expected

@@ -3,7 +3,9 @@
 | test.c:16:20:16:25 | call to malloc | This allocation does not include space to null-terminate the string. |
 | test.c:32:20:32:25 | call to malloc | This allocation does not include space to null-terminate the string. |
 | test.c:49:20:49:25 | call to malloc | This allocation does not include space to null-terminate the string. |
+| test.c:64:20:64:25 | call to malloc | This allocation does not include space to null-terminate the string. |
 | test.cpp:24:35:24:40 | call to malloc | This allocation does not include space to null-terminate the string. |
+| test.cpp:31:35:31:40 | call to malloc | This allocation does not include space to null-terminate the string. |
 | test.cpp:45:28:45:33 | call to malloc | This allocation does not include space to null-terminate the string. |
 | test.cpp:55:28:55:33 | call to malloc | This allocation does not include space to null-terminate the string. |
 | test.cpp:63:28:63:33 | call to malloc | This allocation does not include space to null-terminate the string. |

cpp/ql/test/query-tests/Security/CWE/CWE-131/semmle/NoSpaceForZeroTerminator/test.c

@@ -60,7 +60,7 @@ void good2(char *str) {
 }
 
 void bad3(char *str) {
-    // BAD -- Not allocating space for '\0' terminator [NOT DETECTED]
+    // BAD -- Not allocating space for '\0' terminator
     char *buffer = malloc(strlen(str) * sizeof(char));
     strcpy(buffer, str);
     free(buffer);

cpp/ql/test/query-tests/Security/CWE/CWE-131/semmle/NoSpaceForZeroTerminator/test.cpp

@@ -27,7 +27,7 @@ void bad1(wchar_t *wstr) {
 }
 
 void bad2(wchar_t *wstr) {
-    // BAD -- Not allocating space for '\0' terminator [NOT DETECTED]
+    // BAD -- Not allocating space for '\0' terminator
     wchar_t *wbuffer = (wchar_t *)malloc(wcslen(wstr) * sizeof(wchar_t));
     wcscpy(wbuffer, wstr);
     free(wbuffer);