C++: Exclude results where identity-like functions obscure operations on a variable.

Author: geoffw0

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

@@ -45,22 +45,27 @@ predicate mayAddNullTerminator(Expr e, VariableAccess va) {
     ae.getRValue().getAChild*() = va
   )
   or
-  // Function call: library function, varargs function, function
-  // containing assembler code, or function where the relevant
-  // parameter is potentially added a null terminator.
+  // Function calls...
   exists(Call c, Function f, int i |
     e = c and
     f = c.getTarget() and
     not functionArgumentMustBeNullTerminated(f, i) and
     c.getAnArgumentSubExpr(i) = va
   |
+    // library function
     not f.hasEntryPoint()
     or
+    // function where the relevant parameter is potentially added a null terminator
     mayAddNullTerminator(_, f.getParameter(i).getAnAccess())
     or
+    // varargs function
     f.isVarargs() and i >= f.getNumberOfParameters()
     or
+    // function containing assembler code
     exists(AsmStmt s | s.getEnclosingFunction() = f)
+    or
+    // function where the relevant parameter is returned (leaking it)
+    exists(ReturnStmt rs | rs.getEnclosingFunction() = f and rs.getExpr().getAChild*() = f.getParameter(i).getAnAccess())
   )
   or
   // Call without target (e.g., function pointer call)

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

@@ -17,7 +17,6 @@
 | test.cpp:285:10:285:15 | buffer | Variable $@ may not be null terminated. | test.cpp:282:8:282:13 | buffer | buffer |
 | test.cpp:302:10:302:16 | buffer2 | Variable $@ may not be null terminated. | test.cpp:297:8:297:14 | buffer2 | buffer2 |
 | test.cpp:314:10:314:15 | buffer | Variable $@ may not be null terminated. | test.cpp:310:8:310:13 | buffer | buffer |
-| test.cpp:328:10:328:15 | buffer | Variable $@ may not be null terminated. | test.cpp:325:8:325:13 | buffer | buffer |
 | test.cpp:336:18:336:23 | buffer | Variable $@ may not be null terminated. | test.cpp:335:8:335:13 | buffer | buffer |
 | test.cpp:355:11:355:16 | buffer | Variable $@ may not be null terminated. | test.cpp:350:8:350:13 | buffer | buffer |
 | test.cpp:364:11:364:16 | buffer | Variable $@ may not be null terminated. | test.cpp:359:8:359:13 | buffer | buffer |

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

@@ -325,7 +325,7 @@ void test_strcat()
 		char buffer[1024];
 
 		clearBuffer(id(buffer), 1024);
-		strcat(buffer, "content"); // GOOD [FALSE POSITIVE]
+		strcat(buffer, "content"); // GOOD
 	}
 }