Merge pull request #1535 from geoffw0/nospacezero

CPP: Fix false positives from NoSpaceForZeroTerminator.ql

Author: jbj

change-notes/1.22/analysis-cpp.md

@@ -12,6 +12,7 @@
 | **Query**                  | **Expected impact**    | **Change**                                                       |
 |----------------------------|------------------------|------------------------------------------------------------------|
 | Expression has no effect (`cpp/useless-expression`) | Fewer false positive results | Calls to functions with the `weak` attribute are no longer considered to be side effect free, because they could be overridden with a different implementation at link time. |
+| No space for zero terminator (`cpp/no-space-for-terminator`) | Fewer false positive results | False positives involving strings that are not null-terminated have been excluded. |
 | Suspicious pointer scaling (`cpp/suspicious-pointer-scaling`) | Lower precision | The precision of this query has been reduced to "medium". This coding pattern is used intentionally and safely in a number of real-world projects. Results are no longer displayed on LGTM unless you choose to display them. |
 | Non-constant format string (`cpp/non-constant-format`) | Fewer false positive results | Rewritten using the taint-tracking library. |
 

cpp/ql/src/Security/CWE/CWE-131/NoSpaceForZeroTerminator.ql

@@ -14,6 +14,8 @@
  *       external/cwe/cwe-122
  */
 import cpp
+import semmle.code.cpp.dataflow.DataFlow
+import semmle.code.cpp.models.implementations.Memcpy
 
 class MallocCall extends FunctionCall
 {
@@ -34,6 +36,13 @@ class MallocCall extends FunctionCall
 
 predicate terminationProblem(MallocCall malloc, string msg) {
   malloc.getAllocatedSize() instanceof StrlenCall and
+  not exists(DataFlow::Node def, DataFlow::Node use, FunctionCall fc, MemcpyFunction memcpy, int ix |
+    DataFlow::localFlow(def, use) and
+    def.asExpr() = malloc and
+    fc.getTarget() = memcpy and
+    memcpy.hasArrayOutput(ix) and
+    use.asExpr() = fc.getArgument(ix)
+  ) and
   msg = "This allocation does not include space to null-terminate the string."
 }
 

cpp/ql/test/query-tests/Security/CWE/CWE-131/semmle/NoSpaceForZeroTerminator/test.c

@@ -63,3 +63,15 @@ void good3(char *str) {
     char *buffer = malloc((strlen(str) + 1) * sizeof(char));
     free(buffer);
 }
+
+void *memcpy(void *s1, const void *s2, size_t n);
+
+void good4(char *str) {
+	// GOOD -- allocating a non zero-terminated string
+	int len = strlen(str);
+	char *buffer = malloc(len);
+
+	memcpy(buffer, str, len);
+
+	free(buffer);
+}