CPP: Fix FPs.

Author: geoffw0

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

@@ -8,25 +8,33 @@ import semmle.code.cpp.commons.StringAnalysis
 import semmle.code.cpp.models.interfaces.FormattingFunction
 import semmle.code.cpp.models.implementations.Printf
 
+class PrintfFormatAttribute extends FormatAttribute {
+  PrintfFormatAttribute() {
+    getArchetype() = "printf" or
+    getArchetype() = "__printf__"
+  }
+}
+
 /**
  * A function that can be identified as a `printf` style formatting
  * function by its use of the GNU `format` attribute.
  */
 class AttributeFormattingFunction extends FormattingFunction {
-  FormatAttribute printf_attrib;
-
   override string getCanonicalQLClass() { result = "AttributeFormattingFunction" }
 
   AttributeFormattingFunction() {
-    printf_attrib = getAnAttribute() and
-    (
-      printf_attrib.getArchetype() = "printf" or
-      printf_attrib.getArchetype() = "__printf__"
-    ) and
-    exists(printf_attrib.getFirstFormatArgIndex()) // exclude `vprintf` style format functions
+    exists(PrintfFormatAttribute printf_attrib |
+      printf_attrib = getAnAttribute() and
+      exists(printf_attrib.getFirstFormatArgIndex()) // exclude `vprintf` style format functions
+    )
   }
 
-  override int getFormatParameterIndex() { result = printf_attrib.getFormatIndex() }
+  override int getFormatParameterIndex() {
+    forex(PrintfFormatAttribute printf_attrib |
+      printf_attrib = getAnAttribute() |
+      result = printf_attrib.getFormatIndex()
+    )
+  }
 }
 
 /**

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

@@ -1,13 +1,5 @@
 | NonConstantFormat.c:30:10:30:16 | access to array | The format string argument to printf should be constant to prevent security issues and other potential errors. |
 | NonConstantFormat.c:41:9:41:27 | call to any_random_function | The format string argument to printf should be constant to prevent security issues and other potential errors. |
-| a.c:15:37:15:45 | call to getString | The format string argument to myMultiplyDefinedPrintf should be constant to prevent security issues and other potential errors. |
-| a.c:16:27:16:35 | call to getString | The format string argument to myMultiplyDefinedPrintf should be constant to prevent security issues and other potential errors. |
-| a.c:17:38:17:46 | call to getString | The format string argument to myMultiplyDefinedPrintf2 should be constant to prevent security issues and other potential errors. |
-| a.c:18:28:18:36 | call to getString | The format string argument to myMultiplyDefinedPrintf2 should be constant to prevent security issues and other potential errors. |
-| b.c:12:37:12:45 | call to getString | The format string argument to myMultiplyDefinedPrintf should be constant to prevent security issues and other potential errors. |
-| b.c:13:27:13:35 | call to getString | The format string argument to myMultiplyDefinedPrintf should be constant to prevent security issues and other potential errors. |
-| b.c:14:38:14:46 | call to getString | The format string argument to myMultiplyDefinedPrintf2 should be constant to prevent security issues and other potential errors. |
-| b.c:15:28:15:36 | call to getString | The format string argument to myMultiplyDefinedPrintf2 should be constant to prevent security issues and other potential errors. |
 | nested.cpp:21:23:21:26 | fmt0 | The format string argument to snprintf should be constant to prevent security issues and other potential errors. |
 | nested.cpp:79:32:79:38 | call to get_fmt | The format string argument to diagnostic should be constant to prevent security issues and other potential errors. |
 | nested.cpp:87:18:87:20 | fmt | The format string argument to diagnostic should be constant to prevent security issues and other potential errors. |

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

@@ -12,8 +12,8 @@ char *getString();
 
 void test_custom_printf1()
 {
-  myMultiplyDefinedPrintf("string", getString()); // GOOD [FALSE POSITIVE]
-  myMultiplyDefinedPrintf(getString(), "string"); // BAD
-  myMultiplyDefinedPrintf2("string", getString()); // GOOD (we can't tell which definition is correct so we have to assume this is OK) [FALSE POSITIVE]
-  myMultiplyDefinedPrintf2(getString(), "string"); // GOOD (we can't tell which definition is correct so we have to assume this is OK) [FALSE POSITIVE]
+  myMultiplyDefinedPrintf("string", getString()); // GOOD
+  myMultiplyDefinedPrintf(getString(), "string"); // BAD [NOT DETECTED]
+  myMultiplyDefinedPrintf2("string", getString()); // GOOD (we can't tell which definition is correct so we have to assume this is OK)
+  myMultiplyDefinedPrintf2(getString(), "string"); // GOOD (we can't tell which definition is correct so we have to assume this is OK)
 }

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

@@ -9,8 +9,8 @@ char *getString();
 
 void test_custom_printf2(char *string)
 {
-  myMultiplyDefinedPrintf("string", getString()); // GOOD [FALSE POSITIVE]
-  myMultiplyDefinedPrintf(getString(), "string"); // BAD
-  myMultiplyDefinedPrintf2("string", getString()); // GOOD (we can't tell which definition is correct so we have to assume this is OK) [FALSE POSITIVE]
-  myMultiplyDefinedPrintf2(getString(), "string"); // GOOD (we can't tell which definition is correct so we have to assume this is OK) [FALSE POSITIVE]
+  myMultiplyDefinedPrintf("string", getString()); // GOOD
+  myMultiplyDefinedPrintf(getString(), "string"); // BAD [NOT DETECTED]
+  myMultiplyDefinedPrintf2("string", getString()); // GOOD (we can't tell which definition is correct so we have to assume this is OK)
+  myMultiplyDefinedPrintf2(getString(), "string"); // GOOD (we can't tell which definition is correct so we have to assume this is OK)
 }

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

@@ -1,7 +1,3 @@
-| a.c:13:39:13:42 | 1.0 | This argument should be of type 'char *' but is of type 'double' |
-| a.c:13:39:13:42 | 1.0 | This argument should be of type 'int' but is of type 'double' |
-| a.c:15:40:15:43 | 1.0 | This argument should be of type 'char *' but is of type 'double' |
-| a.c:15:40:15:43 | 1.0 | This argument should be of type 'int' but is of type 'double' |
 | format.h:16:59:16:61 | str | This argument should be of type 'int' but is of type 'char *' |
 | format.h:16:64:16:64 | i | This argument should be of type 'double' but is of type 'int' |
 | format.h:16:67:16:67 | d | This argument should be of type 'char *' but is of type 'double' |

cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_signed_chars/a.c

@@ -10,7 +10,7 @@ void myMultiplyDefinedPrintf2(const char *format, const char *extraArg, ...);
 void test_custom_printf1()
 {
   myMultiplyDefinedPrintf("%i", "%s", 1); // GOOD
-  myMultiplyDefinedPrintf("%i", "%s", 1.0f); // BAD
+  myMultiplyDefinedPrintf("%i", "%s", 1.0f); // BAD [NOT DETECTED]
   myMultiplyDefinedPrintf2("%i", "%s", 1); // GOOD (we can't tell which definition is correct so we have to assume this is OK)
-  myMultiplyDefinedPrintf2("%i", "%s", 1.0f); // GOOD (we can't tell which definition is correct so we have to assume this is OK) [FALSE POSITIVE]
+  myMultiplyDefinedPrintf2("%i", "%s", 1.0f); // GOOD (we can't tell which definition is correct so we have to assume this is OK)
 }