Merge pull request github#11946 from MathiasVP/fix-taint-models-2

Author: MathiasVP

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

@@ -169,19 +169,11 @@ predicate defaultTaintSanitizer(DataFlow::Node node) { none() }
  */
 predicate modeledTaintStep(Operand nodeIn, Instruction nodeOut) {
   exists(CallInstruction call, TaintFunction func, FunctionInput modelIn, FunctionOutput modelOut |
-    (
-      nodeIn = callInput(call, modelIn)
-      or
-      exists(int n |
-        modelIn.isParameterDerefOrQualifierObject(n) and
-        if n = -1
-        then nodeIn = callInput(call, any(InQualifierObject inQualifier))
-        else nodeIn = callInput(call, any(InParameter inParam | inParam.getIndex() = n))
-      )
-    ) and
-    nodeOut = callOutput(call, modelOut) and
     call.getStaticCallTarget() = func and
     func.hasTaintFlow(modelIn, modelOut)
+  |
+    nodeIn = callInput(call, modelIn) and
+    nodeOut = callOutput(call, modelOut)
   )
   or
   // Taint flow from one argument to another and data flow from an argument to a

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

@@ -206,7 +206,7 @@ private class IteratorAssignArithmeticOperatorModel extends IteratorAssignArithm
     input.isReturnValueDeref() and
     output.isParameterDeref(0)
     or
-    input.isParameterDeref(1) and
+    (input.isParameter(1) or input.isParameterDeref(1)) and
     output.isParameterDeref(0)
   }
 }
@@ -305,7 +305,7 @@ private class IteratorAssignArithmeticMemberOperator extends MemberFunction, Dat
     input.isReturnValueDeref() and
     output.isQualifierObject()
     or
-    input.isParameterDeref(0) and
+    (input.isParameter(0) or input.isParameterDeref(0)) and
     output.isQualifierObject()
   }
 }

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

@@ -27,7 +27,12 @@ private class StdSetConstructor extends Constructor, TaintFunction {
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
     // taint flow from any parameter of an iterator type to the qualifier
-    input.isParameterDeref(this.getAnIteratorParameterIndex()) and
+    (
+      // AST dataflow doesn't have indirection for iterators.
+      // Once we deprecate AST dataflow we can delete this first disjunct.
+      input.isParameter(this.getAnIteratorParameterIndex()) or
+      input.isParameterDeref(this.getAnIteratorParameterIndex())
+    ) and
     (
       output.isReturnValue() // TODO: this is only needed for AST data flow, which treats constructors as returning the new object
       or
@@ -45,7 +50,12 @@ private class StdSetInsert extends TaintFunction {
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
     // flow from last parameter to qualifier and return value
     // (where the return value is a pair, this should really flow just to the first part of it)
-    input.isParameterDeref(this.getNumberOfParameters() - 1) and
+    (
+      // AST dataflow doesn't have indirection for iterators.
+      // Once we deprecate AST dataflow we can delete this first disjunct.
+      input.isParameter(this.getNumberOfParameters() - 1) or
+      input.isParameterDeref(this.getNumberOfParameters() - 1)
+    ) and
     (
       output.isQualifierObject() or
       output.isReturnValue()

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

@@ -38,8 +38,7 @@ private class StdBasicStringIterator extends Iterator, Type {
  */
 abstract private class StdStringTaintFunction extends TaintFunction {
   /**
-   * Gets the index of a parameter to this function that is a string (or
-   * character).
+   * Gets the index of a parameter to this function that is a string.
    */
   final int getAStringParameterIndex() {
     exists(Type paramType | paramType = this.getParameter(result).getUnspecifiedType() |
@@ -50,7 +49,14 @@ abstract private class StdStringTaintFunction extends TaintFunction {
       paramType instanceof ReferenceType and
       not paramType.(ReferenceType).getBaseType() =
         this.getDeclaringType().getTemplateArgument(2).(Type).getUnspecifiedType()
-      or
+    )
+  }
+
+  /**
+   * Gets the index of a parameter to this function that is a character.
+   */
+  final int getACharParameterIndex() {
+    exists(Type paramType | paramType = this.getParameter(result).getUnspecifiedType() |
       // i.e. `std::basic_string::CharT`
       paramType = this.getDeclaringType().getTemplateArgument(0).(Type).getUnspecifiedType()
     )
@@ -79,6 +85,7 @@ private class StdStringConstructor extends Constructor, StdStringTaintFunction {
     // taint flow from any parameter of the value type to the returned object
     (
       input.isParameterDeref(this.getAStringParameterIndex()) or
+      input.isParameter(this.getACharParameterIndex()) or
       input.isParameter(this.getAnIteratorParameterIndex())
     ) and
     (
@@ -128,7 +135,7 @@ private class StdStringPush extends StdStringTaintFunction {
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
     // flow from parameter to qualifier
-    input.isParameterDeref(0) and
+    input.isParameter(0) and
     output.isQualifierObject()
   }
 }
@@ -180,6 +187,7 @@ private class StdStringAppend extends StdStringTaintFunction {
     (
       input.isQualifierObject() or
       input.isParameterDeref(this.getAStringParameterIndex()) or
+      input.isParameter(this.getACharParameterIndex()) or
       input.isParameter(this.getAnIteratorParameterIndex())
     ) and
     (
@@ -210,6 +218,7 @@ private class StdStringInsert extends StdStringTaintFunction {
     (
       input.isQualifierObject() or
       input.isParameterDeref(this.getAStringParameterIndex()) or
+      input.isParameter(this.getACharParameterIndex()) or
       input.isParameter(this.getAnIteratorParameterIndex())
     ) and
     (
@@ -236,6 +245,7 @@ private class StdStringAssign extends StdStringTaintFunction {
     // flow from parameter to string itself (qualifier) and return value
     (
       input.isParameterDeref(this.getAStringParameterIndex()) or
+      input.isParameter(this.getACharParameterIndex()) or
       input.isParameter(this.getAnIteratorParameterIndex())
     ) and
     (

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

@@ -158,11 +158,10 @@ abstract class FormattingFunction extends ArrayFunction, TaintFunction {
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
     exists(int arg |
-      (
-        arg = getFormatParameterIndex() or
-        arg >= getFirstFormatArgumentIndex()
-      ) and
-      input.isParameterDeref(arg) and
+      arg = getFormatParameterIndex() or
+      arg >= getFirstFormatArgumentIndex()
+    |
+      (input.isParameterDeref(arg) or input.isParameter(arg)) and
       output.isParameterDeref(getOutputParameterIndex(_))
     )
   }