C++: Add getOutputParameterIndex override to UserDefinedFormattingFunction and accept test changes

Author: MathiasVP

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

@@ -49,6 +49,18 @@ predicate primitiveVariadicFormatter(TopLevelFunction f, int formatParamIndex) {
   )
 }
 
+/**
+ * A standard function such as `vsprintf` that has an output parameter
+ * and a variable argument list of type `va_arg`.
+ */
+private predicate primitiveVariadicFormatterOutput(TopLevelFunction f, int outputParamIndex) {
+  // note: this might look like the regular expression in `primitiveVariadicFormatter`, but
+  // there is one important difference: the [fs] part is not optional, as these classify
+  // the `printf` variants that write to a buffer.
+  // Conveniently, these buffer parameters are all at index 0.
+  f.getName().regexpMatch("_?_?va?[fs]n?w?printf(_s)?(_p)?(_l)?") and outputParamIndex = 0
+}
+
 private predicate callsVariadicFormatter(Function f, int formatParamIndex) {
   exists(FunctionCall fc, int i |
     variadicFormatter(fc.getTarget(), i) and
@@ -57,6 +69,25 @@ private predicate callsVariadicFormatter(Function f, int formatParamIndex) {
   )
 }
 
+private predicate callsVariadicFormatterOutput(Function f, int outputParamIndex) {
+  exists(FunctionCall fc, int i |
+    fc.getEnclosingFunction() = f and
+    variadicFormatterOutput(fc.getTarget(), i) and
+    fc.getArgument(i) = f.getParameter(outputParamIndex).getAnAccess()
+  )
+}
+
+/**
+ * Holds if `f` is a function such as `vprintf` that writes formatted
+ * output to buffer given as a parameter at index `outputParamIndex`, if any.
+ */
+private predicate variadicFormatterOutput(Function f, int outputParamIndex) {
+  primitiveVariadicFormatterOutput(f, outputParamIndex)
+  or
+  not f.isVarargs() and
+  callsVariadicFormatterOutput(f, outputParamIndex)
+}
+
 /**
  * Holds if `f` is a function such as `vprintf` that has a format parameter
  * (at `formatParamIndex`) and a variable argument list of type `va_arg`.
@@ -78,6 +109,8 @@ class UserDefinedFormattingFunction extends FormattingFunction {
   UserDefinedFormattingFunction() { isVarargs() and callsVariadicFormatter(this, _) }
 
   override int getFormatParameterIndex() { callsVariadicFormatter(this, result) }
+
+  override int getOutputParameterIndex() { callsVariadicFormatterOutput(this, result) }
 }
 
 /**

cpp/ql/test/query-tests/Security/CWE/CWE-242/semmle/tests/OverrunWrite.expected

@@ -2,3 +2,4 @@
 | tests.cpp:259:2:259:8 | call to sprintf | This 'call to sprintf' operation requires 17 bytes but the destination is only 10 bytes. |
 | tests.cpp:272:2:272:8 | call to sprintf | This 'call to sprintf' operation requires 9 bytes but the destination is only 8 bytes. |
 | tests.cpp:273:2:273:8 | call to sprintf | This 'call to sprintf' operation requires 9 bytes but the destination is only 8 bytes. |
+| tests.cpp:308:3:308:9 | call to sprintf | This 'call to sprintf' operation requires 9 bytes but the destination is only 8 bytes. |

cpp/ql/test/query-tests/Security/CWE/CWE-242/semmle/tests/tests.cpp

@@ -305,6 +305,6 @@ namespace custom_sprintf_impl {
 	void regression_test1()
 	{
 		char buffer8[8];
-		sprintf(buffer8, "12345678"); // BAD: potential buffer overflow [NOT DETECTED]
+		sprintf(buffer8, "12345678"); // BAD: potential buffer overflow
 	}
 }