Merge pull request #6757 from geoffw0/impropnulltest2

C++: Small improvement to cpp/improper-null-termination

cpp/ql/lib/semmle/code/cpp/commons/NullTermination.qll

@@ -93,6 +93,15 @@ predicate variableMustBeNullTerminated(VariableAccess va) {
       fc.getArgument(i) = va
     )
     or
+    // String argument to a formatting function (such as `printf`)
+    exists(int n, FormatLiteral fl |
+      fc.(FormattingFunctionCall).getConversionArgument(n) = va and
+      fl = fc.(FormattingFunctionCall).getFormat() and
+      fl.getConversionType(n) instanceof PointerType and // `%s`, `%ws` etc
+      not fl.getConversionType(n) instanceof VoidPointerType and // exclude: `%p`
+      not fl.hasPrecision(n) // exclude: `%.*s`
+    )
+    or
     // Call to a wrapper function that requires null termination
     // (not itself adding a null terminator)
     exists(Function wrapper, int i, Parameter p, VariableAccess use |

cpp/ql/test/query-tests/Likely Bugs/Memory Management/ImproperNullTermination/ImproperNullTermination.expected

@@ -23,3 +23,5 @@
 | test.cpp:365:19:365:25 | buffer2 | Variable $@ may not be null terminated. | test.cpp:363:8:363:14 | buffer2 | buffer2 |
 | test.cpp:392:17:392:22 | buffer | Variable $@ may not be null terminated. | test.cpp:390:8:390:13 | buffer | buffer |
 | test.cpp:398:18:398:23 | buffer | Variable $@ may not be null terminated. | test.cpp:396:8:396:13 | buffer | buffer |
+| test.cpp:444:10:444:15 | buffer | Variable $@ may not be null terminated. | test.cpp:442:8:442:13 | buffer | buffer |
+| test.cpp:450:16:450:21 | buffer | Variable $@ may not be null terminated. | test.cpp:448:8:448:13 | buffer | buffer |

cpp/ql/test/query-tests/Likely Bugs/Memory Management/ImproperNullTermination/test.cpp

@@ -433,3 +433,36 @@ void test_read_fread(int read_src, FILE *s)
 		strlen(buffer); // GOOD
 	}
 }
+
+int printf(const char *format, ...);
+
+void test_printf(char *str)
+{
+	{
+		char buffer[1024];
+
+		printf(buffer, ""); // BAD
+	}
+
+	{
+		char buffer[1024];
+
+		printf("%s", buffer); // BAD
+	}
+
+	{
+		size_t len = strlen(str);
+		char *copied_str = (char *)malloc(len);
+
+		memcpy(copied_str, str, len);
+		printf("%s", copied_str); // BAD [NOT DETECTED]
+	}
+
+	{
+		size_t len = strlen(str);
+		char *copied_str = (char *)malloc(len + 1);
+
+		memcpy(copied_str, str, len + 1);
+		printf("%s", copied_str); // GOOD
+	}
+}