Merge branch 'main' into fixHiddenTypesTestGenerator

Author: Benjamin Muskalla

README.md

@@ -4,8 +4,8 @@ This open source repository contains the standard CodeQL libraries and queries t
 
 ## How do I learn CodeQL and run queries?
 
-There is [extensive documentation](https://help.semmle.com/QL/learn-ql/) on getting started with writing CodeQL.
-You can use the [interactive query console](https://lgtm.com/help/lgtm/using-query-console) on LGTM.com or the [CodeQL for Visual Studio Code](https://help.semmle.com/codeql/codeql-for-vscode.html) extension to try out your queries on any open source project that's currently being analyzed.
+There is [extensive documentation](https://codeql.github.com/docs/) on getting started with writing CodeQL.
+You can use the [interactive query console](https://lgtm.com/help/lgtm/using-query-console) on LGTM.com or the [CodeQL for Visual Studio Code](https://codeql.github.com/docs/codeql-for-visual-studio-code/) extension to try out your queries on any open source project that's currently being analyzed.
 
 ## Contributing
 

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/NanAnalysis.qll

@@ -29,8 +29,7 @@ predicate nanExcludingComparison(ComparisonOperation guard, boolean polarity) {
  */
 private predicate excludesNan(RangeSsaDefinition def, VariableAccess v) {
   exists(VariableAccess inCond, ComparisonOperation guard, boolean branch, StackVariable lsv |
-    def.isGuardPhi(inCond, guard, branch) and
-    inCond.getTarget() = lsv and
+    def.isGuardPhi(lsv, inCond, guard, branch) and
     v = def.getAUse(lsv) and
     guard.getAnOperand() = inCond and
     nanExcludingComparison(guard, branch)

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

@@ -94,10 +94,11 @@ class RangeSsaDefinition extends ControlFlowNodeBase {
   predicate isPhiNode(StackVariable v) { exists(RangeSSA x | x.phi_node(v, this.(BasicBlock))) }
 
   /**
+   * DEPRECATED: Use isGuardPhi/4 instead
    * If this definition is a phi node corresponding to a guard,
    * then return the variable access and the guard.
    */
-  predicate isGuardPhi(VariableAccess va, Expr guard, boolean branch) {
+  deprecated predicate isGuardPhi(VariableAccess va, Expr guard, boolean branch) {
     guard_defn(va, guard, this, branch)
   }
 
@@ -142,9 +143,8 @@ class RangeSsaDefinition extends ControlFlowNodeBase {
       // below excludes definitions which can only reach guard phi
       // nodes by going through the corresponding guard.
       not exists(VariableAccess access |
-        v = access.getTarget() and
         pred.contains(access) and
-        this.isGuardPhi(access, _, _)
+        this.isGuardPhi(v, access, _, _)
       )
     )
   }

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

@@ -433,10 +433,7 @@ private predicate exprDependsOnDef(Expr e, RangeSsaDefinition srcDef, StackVaria
 private predicate phiDependsOnDef(
   RangeSsaDefinition phi, StackVariable v, RangeSsaDefinition srcDef, StackVariable srcVar
 ) {
-  exists(VariableAccess access, Expr guard |
-    access = v.getAnAccess() and
-    phi.isGuardPhi(access, guard, _)
-  |
+  exists(VariableAccess access, Expr guard | phi.isGuardPhi(v, access, guard, _) |
     exprDependsOnDef(guard.(ComparisonOperation).getAnOperand(), srcDef, srcVar) or
     exprDependsOnDef(access, srcDef, srcVar)
   )
@@ -1204,8 +1201,7 @@ private float boolConversionUpperBound(Expr expr) {
  */
 private float getPhiLowerBounds(StackVariable v, RangeSsaDefinition phi) {
   exists(VariableAccess access, Expr guard, boolean branch, float defLB, float guardLB |
-    access = v.getAnAccess() and
-    phi.isGuardPhi(access, guard, branch) and
+    phi.isGuardPhi(v, access, guard, branch) and
     lowerBoundFromGuard(guard, access, guardLB, branch) and
     defLB = getFullyConvertedLowerBounds(access)
   |
@@ -1230,8 +1226,7 @@ private float getPhiLowerBounds(StackVariable v, RangeSsaDefinition phi) {
 /** See comment for `getPhiLowerBounds`, above. */
 private float getPhiUpperBounds(StackVariable v, RangeSsaDefinition phi) {
   exists(VariableAccess access, Expr guard, boolean branch, float defUB, float guardUB |
-    access = v.getAnAccess() and
-    phi.isGuardPhi(access, guard, branch) and
+    phi.isGuardPhi(v, access, guard, branch) and
     upperBoundFromGuard(guard, access, guardUB, branch) and
     defUB = getFullyConvertedUpperBounds(access)
   |
@@ -1493,8 +1488,7 @@ private predicate isNEPhi(
   exists(
     ComparisonOperation cmp, boolean branch, Expr linearExpr, Expr rExpr, float p, float q, float r
   |
-    access.getTarget() = v and
-    phi.isGuardPhi(access, cmp, branch) and
+    phi.isGuardPhi(v, access, cmp, branch) and
     eqOpWithSwapAndNegate(cmp, linearExpr, rExpr, false, branch) and
     v.getUnspecifiedType() instanceof IntegralOrEnumType and // Float `!=` is too imprecise
     r = getValue(rExpr).toFloat() and
@@ -1503,8 +1497,7 @@ private predicate isNEPhi(
   )
   or
   exists(Expr op, boolean branch, Expr linearExpr, float p, float q |
-    access.getTarget() = v and
-    phi.isGuardPhi(access, op, branch) and
+    phi.isGuardPhi(v, access, op, branch) and
     eqZeroWithNegate(op, linearExpr, false, branch) and
     v.getUnspecifiedType() instanceof IntegralOrEnumType and // Float `!` is too imprecise
     linearAccess(linearExpr, access, p, q) and
@@ -1524,8 +1517,7 @@ private predicate isUnsupportedGuardPhi(Variable v, RangeSsaDefinition phi, Vari
     or
     eqZeroWithNegate(cmp, _, false, branch)
   |
-    access.getTarget() = v and
-    phi.isGuardPhi(access, cmp, branch) and
+    phi.isGuardPhi(v, access, cmp, branch) and
     not isNEPhi(v, phi, access, _)
   )
 }
@@ -1549,7 +1541,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 |