Merge branch 'main' into rdmarsh2/cpp/ir-synthetic-destructors

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll

@@ -55,29 +55,12 @@ private newtype TIRDataFlowNode =
   TFinalParameterNode(Parameter p, int indirectionIndex) {
     exists(Ssa::FinalParameterUse use |
       use.getParameter() = p and
-      use.getIndirectionIndex() = indirectionIndex and
-      parameterIsRedefined(p)
+      use.getIndirectionIndex() = indirectionIndex
     )
   } or
   TFinalGlobalValue(Ssa::GlobalUse globalUse) or
   TInitialGlobalValue(Ssa::GlobalDef globalUse)
 
-/**
- * Holds if the value of `*p` (or `**p`, `***p`, etc.) is redefined somewhere in the body
- * of the enclosing function of `p`.
- *
- * Only parameters satisfying this predicate will generate a `FinalParameterNode` transferring
- * flow out of the function.
- */
-private predicate parameterIsRedefined(Parameter p) {
-  exists(Ssa::Def def |
-    def.getSourceVariable().getBaseVariable().(Ssa::BaseIRVariable).getIRVariable().getAst() = p and
-    def.getIndirectionIndex() = 0 and
-    def.getIndirection() > 1 and
-    not def.getValue().asInstruction() instanceof InitializeParameterInstruction
-  )
-}
-
 /**
  * An operand that is defined by a `FieldAddressInstruction`.
  */

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll

@@ -4,7 +4,11 @@ private import DataFlowUtil
 private import DataFlowImplCommon as DataFlowImplCommon
 private import semmle.code.cpp.models.interfaces.Allocation as Alloc
 private import semmle.code.cpp.models.interfaces.DataFlow as DataFlow
+private import semmle.code.cpp.models.interfaces.Taint as Taint
+private import semmle.code.cpp.models.interfaces.PartialFlow as PartialFlow
+private import semmle.code.cpp.models.interfaces.FunctionInputsAndOutputs as FIO
 private import semmle.code.cpp.ir.internal.IRCppLanguage
+private import semmle.code.cpp.ir.dataflow.internal.ModelUtil
 private import DataFlowPrivate
 private import ssa0.SsaInternals as SsaInternals0
 import SsaInternalsCommon
@@ -138,12 +142,11 @@ private newtype TDefOrUseImpl =
     isIteratorUse(container, iteratorAddress, _, indirectionIndex)
   } or
   TFinalParameterUse(Parameter p, int indirectionIndex) {
-    // Avoid creating parameter nodes if there is no definitions of the variable other than the initializaion.
-    exists(SsaInternals0::Def def |
-      def.getSourceVariable().getBaseVariable().(BaseIRVariable).getIRVariable().getAst() = p and
-      not def.getValue().asInstruction() instanceof InitializeParameterInstruction and
-      underlyingTypeIsModifiableAt(p.getUnderlyingType(), indirectionIndex)
-    )
+    underlyingTypeIsModifiableAt(p.getUnderlyingType(), indirectionIndex) and
+    // Only create an SSA read for the final use of a parameter if there's
+    // actually a body of the enclosing function. If there's no function body
+    // then we'll never need to flow out of the function anyway.
+    p.getFunction().hasDefinition()
   }
 
 private predicate isGlobalUse(
@@ -796,10 +799,58 @@ private Node getAPriorDefinition(SsaDefOrUse defOrUse) {
   )
 }
 
+private predicate inOut(FIO::FunctionInput input, FIO::FunctionOutput output) {
+  exists(int indirectionIndex |
+    input.isQualifierObject(indirectionIndex) and
+    output.isQualifierObject(indirectionIndex)
+    or
+    exists(int i |
+      input.isParameterDeref(i, indirectionIndex) and
+      output.isParameterDeref(i, indirectionIndex)
+    )
+  )
+}
+
+/**
+ * Holds if there should not be use-use flow out of `n`. That is, `n` is
+ * an out-barrier to use-use flow. This includes:
+ *
+ * - an input to a call that would be assumed to have use-use flow to the same
+ *   argument as an output, but this flow should be blocked because the
+ *   function is modeled with another flow to that output (for example the
+ *   first argument of `strcpy`).
+ * - a conversion that flows to such an input.
+ */
+private predicate modeledFlowBarrier(Node n) {
+  exists(
+    FIO::FunctionInput input, FIO::FunctionOutput output, CallInstruction call,
+    PartialFlow::PartialFlowFunction partialFlowFunc
+  |
+    n = callInput(call, input) and
+    inOut(input, output) and
+    exists(callOutput(call, output)) and
+    partialFlowFunc = call.getStaticCallTarget() and
+    not partialFlowFunc.isPartialWrite(output)
+  |
+    call.getStaticCallTarget().(DataFlow::DataFlowFunction).hasDataFlow(_, output)
+    or
+    call.getStaticCallTarget().(Taint::TaintFunction).hasTaintFlow(_, output)
+  )
+  or
+  exists(Operand operand, Instruction instr, Node n0, int indirectionIndex |
+    modeledFlowBarrier(n0) and
+    nodeHasInstruction(n0, instr, indirectionIndex) and
+    conversionFlow(operand, instr, false, _) and
+    nodeHasOperand(n, operand, indirectionIndex)
+  )
+}
+
 /** Holds if there is def-use or use-use flow from `nodeFrom` to `nodeTo`. */
 predicate ssaFlow(Node nodeFrom, Node nodeTo) {
   exists(Node nFrom, boolean uncertain, SsaDefOrUse defOrUse |
-    ssaFlowImpl(defOrUse, nFrom, nodeTo, uncertain) and nodeFrom != nodeTo
+    ssaFlowImpl(defOrUse, nFrom, nodeTo, uncertain) and
+    not modeledFlowBarrier(nFrom) and
+    nodeFrom != nodeTo
   |
     if uncertain = true then nodeFrom = [nFrom, getAPriorDefinition(defOrUse)] else nodeFrom = nFrom
   )

cpp/ql/lib/semmle/code/cpp/models/implementations/GetDelim.qll

@@ -15,6 +15,8 @@ private class GetDelimFunction extends TaintFunction, AliasFunction, SideEffectF
     i.isParameter(3) and o.isParameterDeref(0)
   }
 
+  override predicate isPartialWrite(FunctionOutput o) { o.isParameterDeref(3) }
+
   override predicate parameterNeverEscapes(int index) { index = [0, 1, 3] }
 
   override predicate parameterEscapesOnlyViaReturn(int index) { none() }

cpp/ql/lib/semmle/code/cpp/models/implementations/Gets.qll

@@ -27,6 +27,8 @@ private class FgetsFunction extends DataFlowFunction, TaintFunction, ArrayFuncti
     output.isReturnValue()
   }
 
+  override predicate isPartialWrite(FunctionOutput output) { output.isParameterDeref(2) }
+
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
     input.isParameter(2) and
     output.isParameterDeref(0)

cpp/ql/lib/semmle/code/cpp/models/implementations/Inet.qll

@@ -20,6 +20,8 @@ private class InetAton extends TaintFunction, ArrayFunction {
     output.isParameterDeref(1)
   }
 
+  override predicate isPartialWrite(FunctionOutput output) { output.isParameterDeref(1) }
+
   override predicate hasArrayInput(int bufParam) { bufParam = 0 }
 
   override predicate hasArrayOutput(int bufParam) { bufParam = 1 }

cpp/ql/lib/semmle/code/cpp/models/implementations/StdContainer.qll

@@ -118,6 +118,8 @@ private class StdSequenceContainerData extends TaintFunction {
     input.isReturnValueDeref() and
     output.isQualifierObject()
   }
+
+  override predicate isPartialWrite(FunctionOutput output) { output.isQualifierObject() }
 }
 
 /**
@@ -147,6 +149,8 @@ private class StdSequenceContainerPushModel extends StdSequenceContainerPush, Ta
     input.isParameterDeref(0) and
     output.isQualifierObject()
   }
+
+  override predicate isPartialWrite(FunctionOutput output) { output.isQualifierObject() }
 }
 
 /**
@@ -207,6 +211,8 @@ private class StdSequenceContainerInsertModel extends StdSequenceContainerInsert
       output.isReturnValue()
     )
   }
+
+  override predicate isPartialWrite(FunctionOutput output) { output.isQualifierObject() }
 }
 
 /**
@@ -263,6 +269,8 @@ private class StdSequenceContainerAt extends TaintFunction {
     input.isReturnValueDeref() and
     output.isQualifierObject()
   }
+
+  override predicate isPartialWrite(FunctionOutput output) { output.isQualifierObject() }
 }
 
 /**
@@ -297,6 +305,8 @@ private class StdSequenceEmplaceModel extends StdSequenceEmplace, TaintFunction
       output.isReturnValue()
     )
   }
+
+  override predicate isPartialWrite(FunctionOutput output) { output.isQualifierObject() }
 }
 
 /**
@@ -335,6 +345,8 @@ private class StdSequenceEmplaceBackModel extends StdSequenceEmplaceBack, TaintF
     input.isParameterDeref([0 .. this.getNumberOfParameters() - 1]) and
     output.isQualifierObject()
   }
+
+  override predicate isPartialWrite(FunctionOutput output) { output.isQualifierObject() }
 }
 
 /**

cpp/ql/lib/semmle/code/cpp/models/implementations/StdMap.qll

@@ -3,6 +3,7 @@
  */
 
 import semmle.code.cpp.models.interfaces.Taint
+import semmle.code.cpp.models.interfaces.DataFlow
 import semmle.code.cpp.models.interfaces.Iterator
 
 /**
@@ -53,6 +54,8 @@ private class StdMapInsert extends TaintFunction {
       output.isReturnValue()
     )
   }
+
+  override predicate isPartialWrite(FunctionOutput output) { output.isQualifierObject() }
 }
 
 /**
@@ -75,6 +78,8 @@ private class StdMapEmplace extends TaintFunction {
     input.isQualifierObject() and
     output.isReturnValue()
   }
+
+  override predicate isPartialWrite(FunctionOutput output) { output.isQualifierObject() }
 }
 
 /**
@@ -102,6 +107,8 @@ private class StdMapTryEmplace extends TaintFunction {
     input.isQualifierObject() and
     output.isReturnValue()
   }
+
+  override predicate isPartialWrite(FunctionOutput output) { output.isQualifierObject() }
 }
 
 /**
@@ -115,6 +122,8 @@ private class StdMapMerge extends TaintFunction {
     input.isParameterDeref(0) and
     output.isQualifierObject()
   }
+
+  override predicate isPartialWrite(FunctionOutput output) { output.isQualifierObject() }
 }
 
 /**
@@ -132,6 +141,8 @@ private class StdMapAt extends TaintFunction {
     input.isReturnValueDeref() and
     output.isQualifierObject()
   }
+
+  override predicate isPartialWrite(FunctionOutput output) { output.isQualifierObject() }
 }
 
 /**

cpp/ql/lib/semmle/code/cpp/models/implementations/StdSet.qll

@@ -61,6 +61,8 @@ private class StdSetInsert extends TaintFunction {
       output.isReturnValue()
     )
   }
+
+  override predicate isPartialWrite(FunctionOutput output) { output.isQualifierObject() }
 }
 
 /**
@@ -82,6 +84,8 @@ private class StdSetEmplace extends TaintFunction {
     input.isQualifierObject() and
     output.isReturnValue()
   }
+
+  override predicate isPartialWrite(FunctionOutput output) { output.isQualifierObject() }
 }
 
 /**
@@ -95,6 +99,8 @@ private class StdSetMerge extends TaintFunction {
     input.isParameterDeref(0) and
     output.isQualifierObject()
   }
+
+  override predicate isPartialWrite(FunctionOutput output) { output.isQualifierObject() }
 }
 
 /**

cpp/ql/lib/semmle/code/cpp/models/implementations/StdString.qll

@@ -129,6 +129,8 @@ private class StdStringDataModel extends StdStringData, StdStringTaintFunction {
     input.isReturnValueDeref() and
     output.isQualifierObject()
   }
+
+  override predicate isPartialWrite(FunctionOutput output) { output.isQualifierObject() }
 }
 
 /**
@@ -142,6 +144,8 @@ private class StdStringPush extends StdStringTaintFunction {
     input.isParameter(0) and
     output.isQualifierObject()
   }
+
+  override predicate isPartialWrite(FunctionOutput output) { output.isQualifierObject() }
 }
 
 /**
@@ -204,6 +208,8 @@ private class StdStringAppend extends StdStringTaintFunction {
     input.isReturnValueDeref() and
     output.isQualifierObject()
   }
+
+  override predicate isPartialWrite(FunctionOutput output) { output.isQualifierObject() }
 }
 
 /**
@@ -237,6 +243,8 @@ private class StdStringInsert extends StdStringTaintFunction {
     input.isReturnValueDeref() and
     output.isQualifierObject()
   }
+
+  override predicate isPartialWrite(FunctionOutput output) { output.isQualifierObject() }
 }
 
 /**
@@ -305,6 +313,8 @@ private class StdStringAt extends StdStringTaintFunction {
     input.isReturnValueDeref() and
     output.isQualifierObject()
   }
+
+  override predicate isPartialWrite(FunctionOutput output) { output.isQualifierObject() }
 }
 
 /**
@@ -338,6 +348,8 @@ private class StdIStreamIn extends DataFlowFunction, TaintFunction {
     input.isReturnValueDeref() and
     output.isQualifierObject()
   }
+
+  override predicate isPartialWrite(FunctionOutput output) { output.isQualifierObject() }
 }
 
 /**
@@ -358,6 +370,8 @@ private class StdIStreamInNonMember extends DataFlowFunction, TaintFunction {
     output.isReturnValueDeref()
   }
 
+  override predicate isPartialWrite(FunctionOutput output) { output.isQualifierObject() }
+
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
     // flow from first parameter to second parameter
     input.isParameterDeref(0) and
@@ -403,6 +417,8 @@ private class StdIStreamRead extends DataFlowFunction, TaintFunction {
     output.isReturnValueDeref()
   }
 
+  override predicate isPartialWrite(FunctionOutput output) { output.isQualifierObject() }
+
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
     // flow from qualifier to first parameter
     input.isQualifierObject() and
@@ -442,6 +458,8 @@ private class StdIStreamPutBack extends DataFlowFunction, TaintFunction {
     output.isReturnValueDeref()
   }
 
+  override predicate isPartialWrite(FunctionOutput output) { output.isQualifierObject() }
+
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
     // flow from first parameter (value or pointer) to qualifier
     input.isParameter(0) and
@@ -478,6 +496,8 @@ private class StdIStreamGetLine extends DataFlowFunction, TaintFunction {
     output.isReturnValueDeref()
   }
 
+  override predicate isPartialWrite(FunctionOutput output) { output.isQualifierObject() }
+
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
     // flow from qualifier to first parameter
     input.isQualifierObject() and
@@ -540,6 +560,8 @@ private class StdOStreamOut extends DataFlowFunction, TaintFunction {
     output.isReturnValueDeref()
   }
 
+  override predicate isPartialWrite(FunctionOutput output) { output.isQualifierObject() }
+
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
     // flow from first parameter (value or pointer) to qualifier
     input.isParameter(0) and
@@ -579,6 +601,8 @@ private class StdOStreamOutNonMember extends DataFlowFunction, TaintFunction {
     output.isReturnValueDeref()
   }
 
+  override predicate isPartialWrite(FunctionOutput output) { output.isParameterDeref(0) }
+
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
     // flow from second parameter to first parameter
     input.isParameterDeref(1) and
@@ -672,6 +696,8 @@ private class StdStreamFunction extends DataFlowFunction, TaintFunction {
     output.isReturnValueDeref()
   }
 
+  override predicate isPartialWrite(FunctionOutput output) { output.isQualifierObject() }
+
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
     // reverse flow from returned reference to the qualifier
     input.isReturnValueDeref() and

cpp/ql/lib/semmle/code/cpp/models/implementations/Strcrement.qll

@@ -36,6 +36,8 @@ private class Strcrement extends ArrayFunction, TaintFunction, SideEffectFunctio
       input.isParameter(index) and output.isReturnValue()
       or
       input.isParameterDeref(index) and output.isReturnValueDeref()
+      or
+      input.isParameterDeref(index) and output.isParameterDeref(index)
     )
   }
 

cpp/ql/lib/semmle/code/cpp/models/interfaces/DataFlow.qll

@@ -10,6 +10,7 @@
 import semmle.code.cpp.Function
 import FunctionInputsAndOutputs
 import semmle.code.cpp.models.Models
+import PartialFlow
 
 /**
  * A library function for which a value is or may be copied from a parameter
@@ -18,7 +19,7 @@ import semmle.code.cpp.models.Models
  * Note that this does not include partial copying of values or partial writes
  * to destinations; that is covered by `TaintModel.qll`.
  */
-abstract class DataFlowFunction extends Function {
+abstract class DataFlowFunction extends PartialFlowFunction {
   /**
    * Holds if data can be copied from the argument, qualifier, or buffer
    * represented by `input` to the return value or buffer represented by

cpp/ql/lib/semmle/code/cpp/models/interfaces/PartialFlow.qll

@@ -0,0 +1,31 @@
+/**
+ * Provides an abstract class to override the implicit assumption that a
+ * dataflow/taint-tracking model always fully override the parameters they are
+ * are modeled as writing to. To use this QL library, create a QL class
+ * extending `PartialFlowFunction` with a characteristic predicate that selects
+ * the function or set of functions you are modeling and override the
+ * `isPartialWrite` predicate.
+ *
+ * Note: Since both `DataFlowFunction` and `TaintFunction` extend this class
+ * you don't need to explicitly add this as a base class if your QL class
+ * already extends either `DataFlowFunction` or `TaintFunction`.
+ */
+
+import semmle.code.cpp.Function
+import FunctionInputsAndOutputs
+import semmle.code.cpp.models.Models
+
+/**
+ * A function that may update part of a `FunctionOutput`.
+ *
+ * For example, the destination argument of `strcat` only updates part of the
+ * argument.
+ */
+abstract class PartialFlowFunction extends Function {
+  /**
+   * Holds if the write to output does not overwrite the entire value that was
+   * there before, or does not do so reliably. For example the destination
+   * argument of `strcat` is modified but not overwritten.
+   */
+  predicate isPartialWrite(FunctionOutput output) { none() }
+}

cpp/ql/lib/semmle/code/cpp/models/interfaces/Taint.qll

@@ -10,6 +10,7 @@
 import semmle.code.cpp.Function
 import FunctionInputsAndOutputs
 import semmle.code.cpp.models.Models
+import PartialFlow
 
 /**
  * A library function for which a taint-tracking library should propagate taint
@@ -23,7 +24,7 @@ import semmle.code.cpp.models.Models
  * altered (for example copying a string with `strncpy`), this is also considered
  * data flow.
  */
-abstract class TaintFunction extends Function {
+abstract class TaintFunction extends PartialFlowFunction {
   /**
    * Holds if data passed into the argument, qualifier, or buffer represented by
    * `input` influences the return value or buffer represented by `output`