C++: Fix readlink FPs.

Author: geoffw0

cpp/ql/src/Likely Bugs/Memory Management/ImproperNullTermination.ql

@@ -29,14 +29,19 @@ class ImproperNullTerminationReachability extends StackVariableReachabilityWithR
   override predicate isSourceActual(ControlFlowNode node, StackVariable v) {
     node = declWithNoInit(v)
     or
-    exists(Call c, int arg |
+    exists(Call c, int bufferArg, int sizeArg |
       c = node and
       (
-        c.getTarget().hasName("readlink") and arg = 1
+        c.getTarget().hasName("readlink") and bufferArg = 1 and sizeArg = 2
         or
-        c.getTarget().hasName("readlinkat") and arg = 2
+        c.getTarget().hasName("readlinkat") and bufferArg = 2 and sizeArg = 3
       ) and
-      c.getArgument(arg).(VariableAccess).getTarget() = v
+      c.getArgument(bufferArg).(VariableAccess).getTarget() = v and
+      (
+        // buffer size parameter likely matches the full buffer size
+        c.getArgument(sizeArg) instanceof SizeofOperator or
+        c.getArgument(sizeArg).getValue().toInt() = v.getType().getSize()
+      )
     )
   }
 

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

@@ -11,10 +11,4 @@
 | test.cpp:130:14:130:19 | buffer | Variable $@ may not be null terminated. | test.cpp:127:7:127:12 | buffer | buffer |
 | test.cpp:139:10:139:15 | buffer | Variable $@ may not be null terminated. | test.cpp:136:8:136:13 | buffer | buffer |
 | test.cpp:147:14:147:19 | buffer | Variable $@ may not be null terminated. | test.cpp:143:8:143:13 | buffer | buffer |
-| test.cpp:154:10:154:15 | buffer | Variable $@ may not be null terminated. | test.cpp:151:8:151:13 | buffer | buffer |
-| test.cpp:162:10:162:15 | buffer | Variable $@ may not be null terminated. | test.cpp:158:8:158:13 | buffer | buffer |
 | test.cpp:170:10:170:15 | buffer | Variable $@ may not be null terminated. | test.cpp:166:8:166:13 | buffer | buffer |
-| test.cpp:186:10:186:15 | buffer | Variable $@ may not be null terminated. | test.cpp:183:9:183:14 | buffer | buffer |
-| test.cpp:194:10:194:15 | buffer | Variable $@ may not be null terminated. | test.cpp:190:9:190:14 | buffer | buffer |
-| test.cpp:201:10:201:15 | buffer | Variable $@ may not be null terminated. | test.cpp:198:9:198:14 | buffer | buffer |
-| test.cpp:209:10:209:15 | buffer | Variable $@ may not be null terminated. | test.cpp:205:9:205:14 | buffer | buffer |

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

@@ -151,15 +151,15 @@ void test_readlink(int fd, const char *path, size_t sz)
 		char buffer[1024] = {0};
 
 		readlink(path, buffer, sizeof(buffer) - 1);
-		strdup(buffer); // GOOD [FALSE POSITIVE]
+		strdup(buffer); // GOOD
 	}
 
 	{
 		char buffer[1024];
 
 		memset(buffer, 0, sizeof(buffer));
 		readlink(path, buffer, sizeof(buffer) - 1);
-		strdup(buffer); // GOOD [FALSE POSITIVE]
+		strdup(buffer); // GOOD
 	}
 
 	{
@@ -183,29 +183,29 @@ void test_readlink(int fd, const char *path, size_t sz)
 		char *buffer = (char *)malloc(1024);
 
 		readlink(path, buffer, 1024);
-		strdup(buffer); // BAD
+		strdup(buffer); // BAD [NOT DETECTED]
 	}
 
 	{
 		char *buffer = (char *)malloc(1024);
 
 		buffer[1023] = 0;
 		readlink(path, buffer, 1023);
-		strdup(buffer); // GOOD [FALSE POSITIVE]
+		strdup(buffer); // GOOD
 	}
 
 	{
 		char *buffer = (char *)malloc(sz);
 
 		readlink(path, buffer, sz);
-		strdup(buffer); // BAD
+		strdup(buffer); // BAD [NOT DETECTED]
 	}
 
 	{
 		char *buffer = (char *)malloc(sz);
 
 		memset(buffer, 0, sz);
 		readlink(path, buffer, sz - 1);
-		strdup(buffer); // GOOD [FALSE POSITIVE]
+		strdup(buffer); // GOOD
 	}
 }