Merge remote-tracking branch 'upstream/rc/1.24' into alloc-size2

Author: geoffw0

cpp/ql/src/Likely Bugs/Protocols/TlsSettingsMisconfiguration.ql

@@ -3,7 +3,7 @@
  * @description Using the TLS or SSLv23 protocol from the boost::asio library, but not disabling deprecated protocols, or disabling minimum-recommended protocols.
  * @kind problem
  * @problem.severity error
- * @id cpp/boost/tls_settings_misconfiguration
+ * @id cpp/boost/tls-settings-misconfiguration
  * @tags security
  */
 

cpp/ql/src/Options.qll

@@ -4,7 +4,7 @@
  *
  * By default they fall back to the reasonable defaults provided in
  * `DefaultOptions.qll`, but by modifying this file, you can customize
- * the standard Semmle analyses to give better results for your project.
+ * the standard analyses to give better results for your project.
  */
 
 import cpp

cpp/ql/src/qlpack.yml

@@ -2,3 +2,4 @@ name: codeql-cpp
 version: 0.0.0
 dbscheme: semmlecode.cpp.dbscheme
 suites: codeql-suites
+extractor: cpp

cpp/ql/src/semmle/code/cpp/Compilation.qll

@@ -21,9 +21,9 @@ private predicate idOf(@compilation x, int y) = equivalenceRelation(id/2)(x, y)
  * Three things happen to each file during a compilation:
  *
  *   1. The file is compiled by a real compiler, such as gcc or VC.
- *   2. The file is parsed by Semmle's C++ front-end.
+ *   2. The file is parsed by the CodeQL C++ front-end.
  *   3. The parsed representation is converted to database tables by
- *      Semmle's extractor.
+ *      the CodeQL extractor.
  *
  * This class provides CPU and elapsed time information for steps 2 and 3,
  * but not for step 1.

cpp/ql/src/semmle/code/cpp/dataflow/RecursionPrevention.qll

@@ -1,6 +1,6 @@
 /**
  * DEPRECATED: Recursion through `DataFlow::Configuration` is impossible in
- * Semmle Core 1.17 and above. There is no need for this module because it's
+ * any supported tooling. There is no need for this module because it's
  * impossible to accidentally depend on recursion through
  * `DataFlow::Configuration` in current releases.
  *

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

@@ -199,7 +199,27 @@ private predicate instructionTaintStep(Instruction i1, Instruction i2) {
   // Flow through pointer dereference
   i2.(LoadInstruction).getSourceAddress() = i1
   or
-  i2.(UnaryInstruction).getUnary() = i1
+  // Flow through partial reads of arrays and unions
+  i2.(LoadInstruction).getSourceValueOperand().getAnyDef() = i1 and
+  not i1.isResultConflated() and
+  (
+    i1.getResultType() instanceof ArrayType or
+    i1.getResultType() instanceof Union
+  )
+  or
+  // Unary instructions tend to preserve enough information in practice that we
+  // want taint to flow through.
+  // The exception is `FieldAddressInstruction`. Together with the rule for
+  // `LoadInstruction` above and for `ChiInstruction` below, flow through
+  // `FieldAddressInstruction` could cause flow into one field to come out an
+  // unrelated field. This would happen across function boundaries, where the IR
+  // would not be able to match loads to stores.
+  i2.(UnaryInstruction).getUnary() = i1 and
+  (
+    not i2 instanceof FieldAddressInstruction
+    or
+    i2.(FieldAddressInstruction).getField().getDeclaringType() instanceof Union
+  )
   or
   // Flow out of definition-by-reference
   i2.(ChiInstruction).getPartial() = i1.(WriteSideEffectInstruction) and
@@ -213,7 +233,7 @@ private predicate instructionTaintStep(Instruction i1, Instruction i2) {
     or
     t instanceof ArrayType
     or
-    // Buffers or unknown size
+    // Buffers of unknown size
     t instanceof UnknownType
   )
   or

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

@@ -70,8 +70,7 @@ private module VirtualDispatch {
         // Call return
         exists(DataFlowCall call, ReturnKind returnKind |
           other = getAnOutNode(call, returnKind) and
-          src.(ReturnNode).getKind() = returnKind and
-          call.getStaticCallTarget() = src.getEnclosingCallable()
+          returnNodeWithKindAndEnclosingCallable(src, returnKind, call.getStaticCallTarget())
         ) and
         allowFromArg = false
         or
@@ -125,6 +124,18 @@ private module VirtualDispatch {
     }
   }
 
+  /**
+   * A ReturnNode with its ReturnKind and its enclosing callable.
+   *
+   * Used to fix a join ordering issue in flowsFrom.
+   */
+  private predicate returnNodeWithKindAndEnclosingCallable(
+    ReturnNode node, ReturnKind kind, DataFlowCallable callable
+  ) {
+    node.getKind() = kind and
+    node.getEnclosingCallable() = callable
+  }
+
   /** Call through a function pointer. */
   private class DataSensitiveExprCall extends DataSensitiveCall {
     DataSensitiveExprCall() { not exists(this.getStaticCallTarget()) }

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

@@ -24,31 +24,86 @@ class ArgumentNode extends InstructionNode {
   DataFlowCall getCall() { this.argumentOf(result, _) }
 }
 
-private newtype TReturnKind = TNormalReturnKind()
+private newtype TReturnKind =
+  TNormalReturnKind() or
+  TIndirectReturnKind(ParameterIndex index)
 
 /**
  * A return kind. A return kind describes how a value can be returned
  * from a callable. For C++, this is simply a function return.
  */
 class ReturnKind extends TReturnKind {
   /** Gets a textual representation of this return kind. */
-  string toString() { result = "return" }
+  abstract string toString();
+}
+
+private class NormalReturnKind extends ReturnKind, TNormalReturnKind {
+  override string toString() { result = "return" }
+}
+
+private class IndirectReturnKind extends ReturnKind, TIndirectReturnKind {
+  ParameterIndex index;
+
+  IndirectReturnKind() { this = TIndirectReturnKind(index) }
+
+  override string toString() { result = "outparam[" + index.toString() + "]" }
 }
 
 /** A data flow node that occurs as the result of a `ReturnStmt`. */
 class ReturnNode extends InstructionNode {
-  ReturnNode() { exists(ReturnValueInstruction ret | this.getInstruction() = ret.getReturnValue()) }
+  Instruction primary;
+
+  ReturnNode() {
+    exists(ReturnValueInstruction ret | instr = ret.getReturnValue() and primary = ret)
+    or
+    exists(ReturnIndirectionInstruction rii |
+      instr = rii.getSideEffectOperand().getAnyDef() and primary = rii
+    )
+  }
 
   /** Gets the kind of this returned value. */
-  ReturnKind getKind() { result = TNormalReturnKind() }
+  abstract ReturnKind getKind();
+}
+
+class ReturnValueNode extends ReturnNode {
+  override ReturnValueInstruction primary;
+
+  override ReturnKind getKind() { result = TNormalReturnKind() }
+}
+
+class ReturnIndirectionNode extends ReturnNode {
+  override ReturnIndirectionInstruction primary;
+
+  override ReturnKind getKind() { result = TIndirectReturnKind(primary.getParameter().getIndex()) }
 }
 
 /** A data flow node that represents the output of a call. */
 class OutNode extends InstructionNode {
-  override CallInstruction instr;
+  OutNode() {
+    instr instanceof CallInstruction or
+    instr instanceof WriteSideEffectInstruction
+  }
 
   /** Gets the underlying call. */
-  DataFlowCall getCall() { result = instr }
+  abstract DataFlowCall getCall();
+
+  abstract ReturnKind getReturnKind();
+}
+
+private class CallOutNode extends OutNode {
+  override CallInstruction instr;
+
+  override DataFlowCall getCall() { result = instr }
+
+  override ReturnKind getReturnKind() { result instanceof NormalReturnKind }
+}
+
+private class SideEffectOutNode extends OutNode {
+  override WriteSideEffectInstruction instr;
+
+  override DataFlowCall getCall() { result = instr.getPrimaryInstruction() }
+
+  override ReturnKind getReturnKind() { result = TIndirectReturnKind(instr.getIndex()) }
 }
 
 /**
@@ -57,7 +112,7 @@ class OutNode extends InstructionNode {
  */
 OutNode getAnOutNode(DataFlowCall call, ReturnKind kind) {
   result.getCall() = call and
-  kind = TNormalReturnKind()
+  result.getReturnKind() = kind
 }
 
 /**

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

@@ -525,7 +525,7 @@ class ReturnValueInstruction extends ReturnInstruction {
   final Instruction getReturnValue() { result = getReturnValueOperand().getDef() }
 }
 
-class ReturnIndirectionInstruction extends Instruction {
+class ReturnIndirectionInstruction extends VariableInstruction {
   ReturnIndirectionInstruction() { getOpcode() instanceof Opcode::ReturnIndirection }
 
   final SideEffectOperand getSideEffectOperand() { result = getAnOperand() }
@@ -535,6 +535,12 @@ class ReturnIndirectionInstruction extends Instruction {
   final AddressOperand getSourceAddressOperand() { result = getAnOperand() }
 
   final Instruction getSourceAddress() { result = getSourceAddressOperand().getDef() }
+
+  /**
+   * Gets the parameter for which this instruction reads the final pointed-to value within the
+   * function.
+   */
+  final Language::Parameter getParameter() { result = var.(IRUserVariable).getVariable() }
 }
 
 class CopyInstruction extends Instruction {

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

@@ -525,7 +525,7 @@ class ReturnValueInstruction extends ReturnInstruction {
   final Instruction getReturnValue() { result = getReturnValueOperand().getDef() }
 }
 
-class ReturnIndirectionInstruction extends Instruction {
+class ReturnIndirectionInstruction extends VariableInstruction {
   ReturnIndirectionInstruction() { getOpcode() instanceof Opcode::ReturnIndirection }
 
   final SideEffectOperand getSideEffectOperand() { result = getAnOperand() }
@@ -535,6 +535,12 @@ class ReturnIndirectionInstruction extends Instruction {
   final AddressOperand getSourceAddressOperand() { result = getAnOperand() }
 
   final Instruction getSourceAddress() { result = getSourceAddressOperand().getDef() }
+
+  /**
+   * Gets the parameter for which this instruction reads the final pointed-to value within the
+   * function.
+   */
+  final Language::Parameter getParameter() { result = var.(IRUserVariable).getVariable() }
 }
 
 class CopyInstruction extends Instruction {