C++: Combine getOutputParameterIndex and isOutputStream.

Author: geoffw0

cpp/ql/src/semmle/code/cpp/commons/Printf.qll

@@ -128,13 +128,13 @@ class UserDefinedFormattingFunction extends FormattingFunction {
 
   override int getFormatParameterIndex() { callsVariadicFormatter(this, _, result, _) }
 
-  override int getOutputParameterIndex() {
-    callsVariadicFormatter(this, _, _, result) and not result = -1
+  override int getOutputParameterIndex(boolean isStream) {
+    callsVariadicFormatter(this, "f", _, result) and isStream = true
+    or
+    callsVariadicFormatter(this, "s", _, result) and isStream = false
   }
 
   override predicate isOutputGlobal() { callsVariadicFormatter(this, "", _, _) }
-
-  override predicate isOutputStream() { callsVariadicFormatter(this, "f", _, _) }
 }
 
 /**

cpp/ql/src/semmle/code/cpp/models/implementations/Printf.qll

@@ -58,9 +58,7 @@ private class Fprintf extends FormattingFunction {
 
   deprecated override predicate isWideCharDefault() { hasGlobalOrStdName("fwprintf") }
 
-  override int getOutputParameterIndex() { result = 0 }
-
-  override predicate isOutputStream() { any() }
+  override int getOutputParameterIndex(boolean isStream) { result = 0 and isStream = true }
 }
 
 /**
@@ -113,7 +111,9 @@ private class Sprintf extends FormattingFunction {
     result = 1
   }
 
-  override int getOutputParameterIndex() { not hasGlobalName("g_strdup_printf") and result = 0 }
+  override int getOutputParameterIndex(boolean isStream) {
+    not hasGlobalName("g_strdup_printf") and result = 0 and isStream = false
+  }
 
   override int getFirstFormatArgumentIndex() {
     if hasGlobalName("__builtin___sprintf_chk")
@@ -168,7 +168,7 @@ private class SnprintfImpl extends Snprintf {
         .getSize() > 1
   }
 
-  override int getOutputParameterIndex() { result = 0 }
+  override int getOutputParameterIndex(boolean isStream) { result = 0 and isStream = false }
 
   override int getFirstFormatArgumentIndex() {
     exists(string name |
@@ -228,7 +228,7 @@ private class StringCchPrintf extends FormattingFunction {
         .getSize() > 1
   }
 
-  override int getOutputParameterIndex() { result = 0 }
+  override int getOutputParameterIndex(boolean isStream) { result = 0 and isStream = false }
 
   override int getSizeParameterIndex() { result = 1 }
 }

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

@@ -109,24 +109,25 @@ abstract class FormattingFunction extends ArrayFunction, TaintFunction {
   }
 
   /**
-   * Gets the position at which the output parameter, if any, occurs. This may
-   * be a buffer that characters are written to if this function behaves like
-   * `sprintf`.  Alternatively it may be a stream that is used for output if
-   * this function behaves like `fprintf` (see `isOutputStream`).
+   * Gets the position at which the output parameter, if any, occurs. If
+   * `isStream` is `true`, the output parameter is a stream (that is, this
+   * function behaves like `fprintf`). If `isStream` is `false`, the output
+   * parameter is a buffer (that is, this function behaves like `sprintf`).
    */
-  int getOutputParameterIndex() { none() }
+  int getOutputParameterIndex(boolean isStream) { none() }
 
   /**
-   * Holds if this function outputs to a global stream such as standard output,
-   * standard error or a system log. For example `printf`.
+   * Gets the position at which the output parameter, if any, occurs.
+   *
+   * DEPRECATED: use `getOutputParameterIndex(boolean isStream)` instead.
    */
-  predicate isOutputGlobal() { none() }
+  deprecated int getOutputParameterIndex() { result = getOutputParameterIndex(_) }
 
   /**
-   * Holds if this function outputs to the stream indicated by
-   * `getOutputParameterIndex()`, that is, this function behaves like `fprintf`.
+   * Holds if this function outputs to a global stream such as standard output,
+   * standard error or a system log. For example `printf`.
    */
-  predicate isOutputStream() { none() }
+  predicate isOutputGlobal() { none() }
 
   /**
    * Gets the position of the first format argument, corresponding with
@@ -156,20 +157,18 @@ abstract class FormattingFunction extends ArrayFunction, TaintFunction {
   }
 
   override predicate hasArrayWithVariableSize(int bufParam, int countParam) {
-    bufParam = getOutputParameterIndex() and
+    bufParam = getOutputParameterIndex(false) and
     countParam = getSizeParameterIndex()
   }
 
   override predicate hasArrayWithUnknownSize(int bufParam) {
-    bufParam = getOutputParameterIndex() and
+    bufParam = getOutputParameterIndex(false) and
     not exists(getSizeParameterIndex())
   }
 
   override predicate hasArrayInput(int bufParam) { bufParam = getFormatParameterIndex() }
 
-  override predicate hasArrayOutput(int bufParam) {
-    bufParam = getOutputParameterIndex() and not isOutputStream()
-  }
+  override predicate hasArrayOutput(int bufParam) { bufParam = getOutputParameterIndex(false) }
 
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
     exists(int arg |
@@ -178,7 +177,7 @@ abstract class FormattingFunction extends ArrayFunction, TaintFunction {
         arg >= getFirstFormatArgumentIndex()
       ) and
       input.isParameterDeref(arg) and
-      output.isParameterDeref(getOutputParameterIndex())
+      output.isParameterDeref(getOutputParameterIndex(_))
     )
   }
 }

cpp/ql/src/semmle/code/cpp/security/BufferWrite.qll

@@ -229,7 +229,7 @@ class SprintfBW extends BufferWriteCall {
     result = this.(FormattingFunctionCall).getFormatArgument(_)
   }
 
-  override Expr getDest() { result = getArgument(f.getOutputParameterIndex()) }
+  override Expr getDest() { result = getArgument(f.getOutputParameterIndex(false)) }
 
   override int getMaxData() {
     exists(FormatLiteral fl |

cpp/ql/src/semmle/code/cpp/security/FileWrite.qll

@@ -143,9 +143,8 @@ private predicate fileWrite(Call write, Expr source, Expr dest) {
     )
     or
     // fprintf
-    f.(FormattingFunction).isOutputStream() and
     s >= f.(FormattingFunction).getFormatParameterIndex() and
-    d = f.(FormattingFunction).getOutputParameterIndex()
+    d = f.(FormattingFunction).getOutputParameterIndex(true)
   )
   or
   // file stream using '<<', 'put' or 'write'

cpp/ql/src/semmle/code/cpp/security/TaintTrackingImpl.qll

@@ -478,7 +478,7 @@ private predicate copyValueBetweenArguments(Function f, int sourceArg, int destA
   or
   exists(FormattingFunction ff | ff = f |
     sourceArg in [ff.getFormatParameterIndex() .. maxArgIndex(f)] and
-    destArg = ff.getOutputParameterIndex()
+    destArg = ff.getOutputParameterIndex(false)
   )
 }