Merge remote-tracking branch 'upstream/main' into relax_memberMayBeVarSize

Author: andersfugmann

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

@@ -93,6 +93,15 @@ predicate variableMustBeNullTerminated(VariableAccess va) {
       fc.getArgument(i) = va
     )
     or
+    // String argument to a formatting function (such as `printf`)
+    exists(int n, FormatLiteral fl |
+      fc.(FormattingFunctionCall).getConversionArgument(n) = va and
+      fl = fc.(FormattingFunctionCall).getFormat() and
+      fl.getConversionType(n) instanceof PointerType and // `%s`, `%ws` etc
+      not fl.getConversionType(n) instanceof VoidPointerType and // exclude: `%p`
+      not fl.hasPrecision(n) // exclude: `%.*s`
+    )
+    or
     // Call to a wrapper function that requires null termination
     // (not itself adding a null terminator)
     exists(Function wrapper, int i, Parameter p, VariableAccess use |

cpp/ql/lib/semmle/code/cpp/ir/dataflow/DefaultTaintTracking.qll

@@ -550,6 +550,39 @@ module TaintedWithPath {
     )
   }
 
+  /**
+   * Holds if there is flow from `arg` to `out` across a call that can by summarized by the flow
+   * from `par` to `ret` within it, in the graph of data flow path explanations.
+   */
+  query predicate subpaths(PathNode arg, PathNode par, PathNode ret, PathNode out) {
+    DataFlow3::PathGraph::subpaths(arg.(WrapPathNode).inner(), par.(WrapPathNode).inner(),
+      ret.(WrapPathNode).inner(), out.(WrapPathNode).inner())
+    or
+    // To avoid showing trivial-looking steps, we _replace_ the last node instead
+    // of adding an edge out of it.
+    exists(WrapPathNode sinkNode |
+      DataFlow3::PathGraph::subpaths(arg.(WrapPathNode).inner(), par.(WrapPathNode).inner(),
+        ret.(WrapPathNode).inner(), sinkNode.inner()) and
+      out.(FinalPathNode).inner() = adjustedSink(sinkNode.inner().getNode())
+    )
+    or
+    // Same for the first node
+    exists(WrapPathNode sourceNode |
+      DataFlow3::PathGraph::subpaths(sourceNode.inner(), par.(WrapPathNode).inner(),
+        ret.(WrapPathNode).inner(), out.(WrapPathNode).inner()) and
+      sourceNode.inner().getNode() = getNodeForExpr(arg.(InitialPathNode).inner())
+    )
+    or
+    // Finally, handle the case where the path goes directly from a source to a
+    // sink, meaning that they both need to be translated.
+    exists(WrapPathNode sinkNode, WrapPathNode sourceNode |
+      DataFlow3::PathGraph::subpaths(sourceNode.inner(), par.(WrapPathNode).inner(),
+        ret.(WrapPathNode).inner(), sinkNode.inner()) and
+      sourceNode.inner().getNode() = getNodeForExpr(arg.(InitialPathNode).inner()) and
+      out.(FinalPathNode).inner() = adjustedSink(sinkNode.inner().getNode())
+    )
+  }
+
   /** Holds if `n` is a node in the graph of data flow path explanations. */
   query predicate nodes(PathNode n, string key, string val) {
     key = "semmle.label" and val = n.toString()

cpp/ql/lib/semmle/code/cpp/ir/implementation/aliased_ssa/Instruction.qll

@@ -1856,12 +1856,12 @@ class InitializeDynamicAllocationInstruction extends SideEffectInstruction {
   }
 
   /**
-   * Gets the address of the allocation this instruction is initializing.
+   * Gets the operand that represents the address of the allocation this instruction is initializing.
    */
   final AddressOperand getAllocationAddressOperand() { result = getAnOperand() }
 
   /**
-   * Gets the operand for the allocation this instruction is initializing.
+   * Gets the address for the allocation this instruction is initializing.
    */
   final Instruction getAllocationAddress() { result = getAllocationAddressOperand().getDef() }
 }

cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/Instruction.qll

@@ -1856,12 +1856,12 @@ class InitializeDynamicAllocationInstruction extends SideEffectInstruction {
   }
 
   /**
-   * Gets the address of the allocation this instruction is initializing.
+   * Gets the operand that represents the address of the allocation this instruction is initializing.
    */
   final AddressOperand getAllocationAddressOperand() { result = getAnOperand() }
 
   /**
-   * Gets the operand for the allocation this instruction is initializing.
+   * Gets the address for the allocation this instruction is initializing.
    */
   final Instruction getAllocationAddress() { result = getAllocationAddressOperand().getDef() }
 }

cpp/ql/lib/semmle/code/cpp/ir/implementation/unaliased_ssa/Instruction.qll

@@ -1856,12 +1856,12 @@ class InitializeDynamicAllocationInstruction extends SideEffectInstruction {
   }
 
   /**
-   * Gets the address of the allocation this instruction is initializing.
+   * Gets the operand that represents the address of the allocation this instruction is initializing.
    */
   final AddressOperand getAllocationAddressOperand() { result = getAnOperand() }
 
   /**
-   * Gets the operand for the allocation this instruction is initializing.
+   * Gets the address for the allocation this instruction is initializing.
    */
   final Instruction getAllocationAddress() { result = getAllocationAddressOperand().getDef() }
 }

cpp/ql/lib/semmle/code/cpp/rangeanalysis/SimpleRangeAnalysis.qll

@@ -1549,7 +1549,8 @@ private float getGuardedUpperBound(VariableAccess guardedAccess) {
     // that there is one predecessor, albeit somewhat conservative.
     exists(unique(BasicBlock b | b = def.(BasicBlock).getAPredecessor())) and
     guardedAccess = def.getAUse(v) and
-    result = max(float ub | upperBoundFromGuard(guard, guardVa, ub, branch))
+    result = max(float ub | upperBoundFromGuard(guard, guardVa, ub, branch)) and
+    not convertedExprMightOverflow(guard.getAChild+())
   )
 }
 

cpp/ql/test/library-tests/rangeanalysis/SimpleRangeAnalysis/lowerBound.expected

@@ -599,6 +599,10 @@
 | test.c:675:7:675:7 | y | -2147483648 |
 | test.c:684:7:684:7 | x | -2147483648 |
 | test.c:689:7:689:7 | x | -2147483648 |
+| test.c:696:8:696:8 | x | 2147483647 |
+| test.c:696:12:696:12 | y | 256 |
+| test.c:697:9:697:9 | x | 2147483647 |
+| test.c:698:9:698:9 | y | 256 |
 | test.cpp:10:7:10:7 | b | -2147483648 |
 | test.cpp:11:5:11:5 | x | -2147483648 |
 | test.cpp:13:10:13:10 | x | -2147483648 |

cpp/ql/test/library-tests/rangeanalysis/SimpleRangeAnalysis/test.c

@@ -689,3 +689,12 @@ void test(int x) {
   out(x);
   goto label;
 }
+
+void test_overflow() {
+  const int x = 2147483647; // 2^31-1
+  const int y = 256;
+  if ((x + y) <= 512) {
+    out(x);
+    out(y);
+  }
+}

cpp/ql/test/library-tests/rangeanalysis/SimpleRangeAnalysis/upperBound.expected

@@ -599,6 +599,10 @@
 | test.c:675:7:675:7 | y | 2147483647 |
 | test.c:684:7:684:7 | x | 2147483647 |
 | test.c:689:7:689:7 | x | 15 |
+| test.c:696:8:696:8 | x | 2147483647 |
+| test.c:696:12:696:12 | y | 256 |
+| test.c:697:9:697:9 | x | 2147483647 |
+| test.c:698:9:698:9 | y | 256 |
 | test.cpp:10:7:10:7 | b | 2147483647 |
 | test.cpp:11:5:11:5 | x | 2147483647 |
 | test.cpp:13:10:13:10 | x | 2147483647 |

cpp/ql/test/query-tests/Likely Bugs/Memory Management/ImproperNullTermination/ImproperNullTermination.expected

@@ -23,3 +23,5 @@
 | test.cpp:365:19:365:25 | buffer2 | Variable $@ may not be null terminated. | test.cpp:363:8:363:14 | buffer2 | buffer2 |
 | test.cpp:392:17:392:22 | buffer | Variable $@ may not be null terminated. | test.cpp:390:8:390:13 | buffer | buffer |
 | test.cpp:398:18:398:23 | buffer | Variable $@ may not be null terminated. | test.cpp:396:8:396:13 | buffer | buffer |
+| test.cpp:444:10:444:15 | buffer | Variable $@ may not be null terminated. | test.cpp:442:8:442:13 | buffer | buffer |
+| test.cpp:450:16:450:21 | buffer | Variable $@ may not be null terminated. | test.cpp:448:8:448:13 | buffer | buffer |