Merge pull request #17553 from github/calumgrant/bmn/wrong-number-of-format-arguments

C++: Remove FPs in cpp/wrong-number-format-arguments due to BMN

Author: calumgrant

cpp/ql/lib/semmle/code/cpp/Function.qll

@@ -651,7 +651,8 @@ class FunctionDeclarationEntry extends DeclarationEntry, @fun_decl {
 
   /**
    * Holds if this declaration is an implicit function declaration, that is,
-   * where a function is used before it is declared (under older C standards).
+   * where a function is used before it is declared (under older C standards,
+   * or when there were parse errors).
    */
   predicate isImplicit() { fun_implicit(underlyingElement(this)) }
 

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

@@ -118,19 +118,34 @@ abstract class FormattingFunction extends ArrayFunction, TaintFunction {
 
   /**
    * Gets the position of the first format argument, corresponding with
-   * the first format specifier in the format string.
+   * the first format specifier in the format string. We ignore all
+   * implicit function definitions.
    */
   int getFirstFormatArgumentIndex() {
-    result = this.getNumberOfParameters() and
-    // the formatting function either has a definition in the snapshot, or all
+    // The formatting function either has a definition in the snapshot, or all
     // `DeclarationEntry`s agree on the number of parameters (otherwise we don't
     // really know the correct number)
-    (
-      this.hasDefinition()
-      or
-      forall(FunctionDeclarationEntry fde | fde = this.getADeclarationEntry() |
-        result = fde.getNumberOfParameters()
-      )
+    if this.hasDefinition()
+    then result = this.getDefinition().getNumberOfParameters()
+    else result = this.getNumberOfExplicitParameters()
+  }
+
+  /**
+   * Gets a non-implicit function declaration entry.
+   */
+  private FunctionDeclarationEntry getAnExplicitDeclarationEntry() {
+    result = this.getADeclarationEntry() and
+    not result.isImplicit()
+  }
+
+  /**
+   * Gets the number of parameters, excluding any parameters that have been defined
+   * from implicit function declarations. If there is some inconsistency in the number
+   * of parameters, then don't return anything.
+   */
+  private int getNumberOfExplicitParameters() {
+    forex(FunctionDeclarationEntry fde | fde = this.getAnExplicitDeclarationEntry() |
+      result = fde.getNumberOfParameters()
     )
   }
 

cpp/ql/src/change-notes/2024-09-26-wrong-number-format-arguments.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Fixed false positives in the `cpp/wrong-number-format-arguments` ("Too few arguments to formatting function") query when the formatting function has been declared implicitly.

cpp/ql/test/query-tests/Likely Bugs/Format/WrongNumberOfFormatArguments/WrongNumberOfFormatArguments.expected

@@ -10,3 +10,4 @@
 | test.c:15:2:15:7 | call to printf | Format for printf expects 3 arguments but given 2 |
 | test.c:19:2:19:7 | call to printf | Format for printf expects 2 arguments but given 1 |
 | test.c:29:3:29:8 | call to printf | Format for printf expects 2 arguments but given 1 |
+| test.c:53:2:53:10 | call to my_logger | Format for my_logger expects 3 arguments but given 2 |

cpp/ql/test/query-tests/Likely Bugs/Format/WrongNumberOfFormatArguments/custom_printf.cpp

@@ -44,3 +44,6 @@ void test_custom_printf2()
 	printf("",           "%i %i",  100, 200); // GOOD
 	printf("%i %i",      ""        );         // GOOD
 }
+
+extern "C" void my_logger(int param, char *fmt, ...) __attribute__((format(printf, 2, 3))) {}
+

cpp/ql/test/query-tests/Likely Bugs/Format/WrongNumberOfFormatArguments/test.c

@@ -46,4 +46,12 @@ void test(int i, const char *str)
 	printf("%Y", 1, 2); // GOOD (unknown format character, this might be correct)
 	printf("%1.1Y", 1, 2); // GOOD (unknown format character, this might be correct)
 	printf("%*.*Y", 1, 2); // GOOD (unknown format character, this might be correct)
+
+	// Implicit logger function declaration
+	my_logger(0, "%i %i %i %i %i %i\n", 1, 2, 3, 4, 5, 6); // GOOD
+	my_logger(0, "%i %i %i\n", 1, 2, 3); // GOOD
+	my_logger(0, "%i %i %i\n", 1, 2); // BAD (too few format arguments)
 }
+
+// A spurious definition of my_logger
+extern void my_logger(int param, char *fmt, int, int, int, int, int);