C++: Fix conflation in 'Pure' models.

Author: MathiasVP

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

@@ -1,5 +1,6 @@
 import semmle.code.cpp.models.interfaces.ArrayFunction
 import semmle.code.cpp.models.interfaces.Taint
+import semmle.code.cpp.models.interfaces.DataFlow
 import semmle.code.cpp.models.interfaces.Alias
 import semmle.code.cpp.models.interfaces.SideEffect
 
@@ -8,7 +9,7 @@ import semmle.code.cpp.models.interfaces.SideEffect
  * guaranteed to be side-effect free.
  */
 private class PureStrFunction extends AliasFunction, ArrayFunction, TaintFunction,
-  SideEffectFunction
+  SideEffectFunction, DataFlowFunction
 {
   PureStrFunction() {
     this.hasGlobalOrStdOrBslName([
@@ -26,22 +27,43 @@ private class PureStrFunction extends AliasFunction, ArrayFunction, TaintFunctio
   }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
-    exists(ParameterIndex i |
+    exists(ParameterIndex i | exists(this.getParameter(i)) |
+      // For these functions we add taint flow according to the following rules:
+      // 1. If the parameter is of a pointer type then there is taint from the
+      // indirection of the parameter. Otherwise, there is taint from the
+      // parameter.
+      // 2. If the return value is of a pointer type then there is taint to the
+      // indirection of the return. Otherwise, there is taint to the return.
       (
-        input.isParameter(i) and
-        exists(this.getParameter(i))
-        or
-        input.isParameterDeref(i) and
-        this.getParameter(i).getUnspecifiedType() instanceof PointerType
+        if this.getParameter(i).getUnspecifiedType() instanceof PointerType
+        then input.isParameterDeref(i)
+        else input.isParameter(i)
       ) and
       // Functions that end with _l also take a locale argument (always as the last argument),
       // and we don't want taint from those arguments.
       (not this.getName().matches("%\\_l") or exists(this.getParameter(i + 1)))
     ) and
     (
-      output.isReturnValueDeref() and
-      this.getUnspecifiedType() instanceof PointerType
-      or
+      if this.getUnspecifiedType() instanceof PointerType
+      then output.isReturnValueDeref()
+      else output.isReturnValue()
+    )
+    or
+    // If there is taint flow from *input to *output then there is also taint
+    // flow from input to output.
+    this.hasTaintFlow(input.getIndirectionInput(), output.getIndirectionOutput()) and
+    // no need to add taint-flow if we already have dataflow
+    not this.hasDataFlow(input, output)
+  }
+
+  override predicate hasDataFlow(FunctionInput input, FunctionOutput output) {
+    exists(int i |
+      input.isParameter(i) and
+      // see the comment in `hasTaintFlow` for an explanation
+      (not this.getName().matches("%\\_l") or exists(this.getParameter(i + 1))) and
+      // These functions always return the same pointer as they are given
+      this.hasGlobalOrStdOrBslName([strrev(), strlwr(), strupr()]) and
+      this.getParameter(i).getUnspecifiedType() instanceof PointerType and
       output.isReturnValue()
     )
   }