Merge pull request #10833 from MathiasVP/repair-badly-bounded-write-2

C++: Prepare `Buffer.qll` for IR-based use-use dataflow

cpp/ql/lib/semmle/code/cpp/commons/Buffer.qll

@@ -25,10 +25,24 @@ predicate memberMayBeVarSize(Class c, MemberVariable v) {
 }
 
 /**
- * Get the size in bytes of the buffer pointed to by an expression (if this can be determined).
+ * Gets the expression associated with `n`. Unlike `n.asExpr()` this also gets the
+ * expression underlying an indirect dataflow node.
  */
-language[monotonicAggregates]
-int getBufferSize(Expr bufferExpr, Element why) {
+private Expr getExpr(DataFlow::Node n, boolean isIndirect) {
+  result = n.asExpr() and isIndirect = false
+  or
+  result = n.asIndirectExpr() and isIndirect = true
+}
+
+private DataFlow::Node exprNode(Expr e, boolean isIndirect) { e = getExpr(result, isIndirect) }
+
+/**
+ * Holds if `bufferExpr` is an allocation-like expression.
+ *
+ * This includes both actual allocations, as well as various operations that return a pointer to
+ * stack-allocated objects.
+ */
+private int isSource(Expr bufferExpr, Element why) {
   exists(Variable bufferVar | bufferVar = bufferExpr.(VariableAccess).getTarget() |
     // buffer is a fixed size array
     result = bufferVar.getUnspecifiedType().(ArrayType).getSize() and
@@ -46,42 +60,12 @@ int getBufferSize(Expr bufferExpr, Element why) {
     ) and
     result = why.(Expr).getType().(ArrayType).getSize() and
     not exists(bufferVar.getUnspecifiedType().(ArrayType).getSize())
-    or
-    exists(Class parentClass, VariableAccess parentPtr, int bufferSize |
-      // buffer is the parentPtr->bufferVar of a 'variable size struct'
-      memberMayBeVarSize(parentClass, bufferVar) and
-      why = bufferVar and
-      parentPtr = bufferExpr.(VariableAccess).getQualifier() and
-      parentPtr.getTarget().getUnspecifiedType().(PointerType).getBaseType() = parentClass and
-      (
-        if exists(bufferVar.getType().getSize())
-        then bufferSize = bufferVar.getType().getSize()
-        else bufferSize = 0
-      ) and
-      result = getBufferSize(parentPtr, _) + bufferSize - parentClass.getSize()
-    )
   )
   or
   // buffer is a fixed size dynamic allocation
   result = bufferExpr.(AllocationExpr).getSizeBytes() and
   why = bufferExpr
   or
-  exists(DataFlow::ExprNode bufferExprNode |
-    // dataflow (all sources must be the same size)
-    bufferExprNode = DataFlow::exprNode(bufferExpr) and
-    result =
-      unique(Expr def |
-        DataFlow::localFlowStep(DataFlow::exprNode(def), bufferExprNode)
-      |
-        getBufferSize(def, _)
-      ) and
-    // find reason
-    exists(Expr def | DataFlow::localFlowStep(DataFlow::exprNode(def), bufferExprNode) |
-      why = def or
-      exists(getBufferSize(def, why))
-    )
-  )
-  or
   exists(Type bufferType |
     // buffer is the address of a variable
     why = bufferExpr.(AddressOfExpr).getAddressable() and
@@ -100,3 +84,117 @@ int getBufferSize(Expr bufferExpr, Element why) {
     result = bufferType.getSize()
   )
 }
+
+/** Holds if the value of `n2 + delta` may be equal to the value of `n1`. */
+private predicate localFlowIncrStep(DataFlow::Node n1, DataFlow::Node n2, int delta) {
+  DataFlow::localFlowStep(n1, n2) and
+  (
+    exists(IncrementOperation incr |
+      n1.asIndirectExpr() = incr.getOperand() and
+      delta = -1
+    )
+    or
+    exists(DecrementOperation decr |
+      n1.asIndirectExpr() = decr.getOperand() and
+      delta = 1
+    )
+    or
+    exists(AddExpr add, Expr e1, Expr e2 |
+      add.hasOperands(e1, e2) and
+      n1.asIndirectExpr() = e1 and
+      delta = -e2.getValue().toInt()
+    )
+    or
+    exists(SubExpr add, Expr e1, Expr e2 |
+      add.hasOperands(e1, e2) and
+      n1.asIndirectExpr() = e1 and
+      delta = e2.getValue().toInt()
+    )
+  )
+}
+
+/**
+ * Holds if `n1` may flow to `n2` without passing through any back-edges.
+ *
+ * Back-edges are excluded to prevent infinite loops on examples like:
+ * ```
+ * while(true) { ++n; }
+ * ```
+ * which, when used in `localFlowStepRec`, would create infinite loop that continuously
+ * increments the `delta` parameter.
+ */
+private predicate localFlowNotIncrStep(DataFlow::Node n1, DataFlow::Node n2) {
+  not localFlowIncrStep(n1, n2, _) and
+  DataFlow::localFlowStep(n1, n2) and
+  not n1 = n2.(DataFlow::SsaPhiNode).getAnInput(true)
+}
+
+private predicate localFlowToExprStep(DataFlow::Node n1, DataFlow::Node n2) {
+  not exists([n1.asExpr(), n1.asIndirectExpr()]) and
+  localFlowNotIncrStep(n1, n2)
+}
+
+/** Holds if `mid2 + delta` may be equal to `n1`. */
+private predicate localFlowStepRec0(DataFlow::Node n1, DataFlow::Node mid2, int delta) {
+  exists(DataFlow::Node mid1, int d1, int d2 |
+    // Or we take a number of steps that adds `d1` to the pointer
+    localFlowStepRec(n1, mid1, d1) and
+    // followed by a step that adds `d2` to the pointer
+    localFlowIncrStep(mid1, mid2, d2) and
+    delta = d1 + d2
+  )
+}
+
+/** Holds if `n2 + delta` may be equal to `n1`. */
+private predicate localFlowStepRec(DataFlow::Node n1, DataFlow::Node n2, int delta) {
+  // Either we take one or more steps that doesn't modify the size of the buffer
+  localFlowNotIncrStep+(n1, n2) and
+  delta = 0
+  or
+  exists(DataFlow::Node mid2 |
+    // Or we step from `n1` to `mid2 + delta`
+    localFlowStepRec0(n1, mid2, delta) and
+    // and finally to the next `ExprNode`.
+    localFlowToExprStep*(mid2, n2)
+  )
+}
+
+/**
+ * Holds if `e2` is an expression that is derived from `e1` such that if `e1[n]` is a
+ * well-defined expression for some number `n`, then `e2[n + delta]` is also a well-defined
+ * expression.
+ */
+private predicate step(Expr e1, Expr e2, int delta) {
+  exists(Variable bufferVar, Class parentClass, VariableAccess parentPtr, int bufferSize |
+    e1 = parentPtr
+  |
+    bufferVar = e2.(VariableAccess).getTarget() and
+    // buffer is the parentPtr->bufferVar of a 'variable size struct'
+    memberMayBeVarSize(parentClass, bufferVar) and
+    parentPtr = e2.(VariableAccess).getQualifier() and
+    parentPtr.getTarget().getUnspecifiedType().(PointerType).getBaseType() = parentClass and
+    (
+      if exists(bufferVar.getType().getSize())
+      then bufferSize = bufferVar.getType().getSize()
+      else bufferSize = 0
+    ) and
+    delta = bufferSize - parentClass.getSize()
+  )
+  or
+  exists(boolean isIndirect |
+    localFlowStepRec(exprNode(e1, isIndirect), exprNode(e2, isIndirect), delta)
+  )
+}
+
+/**
+ * Get the size in bytes of the buffer pointed to by an expression (if this can be determined).
+ */
+int getBufferSize(Expr bufferExpr, Element why) {
+  result = isSource(bufferExpr, why)
+  or
+  exists(Expr e0, int delta, int size |
+    size = getBufferSize(e0, why) and
+    delta = unique(int cand | step(e0, bufferExpr, cand) | cand) and
+    result = size + delta
+  )
+}