Merge pull request #4805 from geoffw0/sscanf

C++: Refine examples and tests for cpp/memory-unsafe-function-scan (experimental) query

Author: Robert Marsh

cpp/ql/src/experimental/Security/CWE/CWE-120/MemoryUnsafeFunctionScan.cpp

@@ -13,9 +13,10 @@ int main(int argc, char **argv)
     char buf1[10];
     scanf("%s", buf1);
 
-    // GOOD, length is specified. The length should be one less than the size of the buffer, since the last character is the NULL terminator.
-    char buf2[10];
-    sscanf(buf2, "%9s");
+    // GOOD, length is specified. The length should be one less than the size of the destination buffer, since the last character is the NULL terminator.
+    char buf2[20];
+    char buf3[10];
+    sscanf(buf2, "%9s", buf3);
 
     // BAD, do not use scanf without specifying a length first
     char file[10];

cpp/ql/test/experimental/query-tests/Security/CWE/semmle/tests/MemoryUnsafeFunctionScan.cpp

@@ -1,5 +1,10 @@
 ///// Library routines /////
 
+typedef unsigned long size_t;
+void *malloc(size_t size);
+
+size_t strlen(const char *s);
+
 int scanf(const char *format, ...);
 int sscanf(const char *str, const char *format, ...);
 int fscanf(const char *str, const char *format, ...);
@@ -13,13 +18,23 @@ int main(int argc, char **argv)
     char buf1[10];
     scanf("%s", buf1);
 
-    // GOOD, length is specified
-    char buf2[10];
-    sscanf(buf2, "%9s");
+    // GOOD, length is specified. The length should be one less than the size of the destination buffer, since the last character is the NULL terminator.
+    char buf2[20];
+    char buf3[10];
+    sscanf(buf2, "%9s", buf3);
 
     // BAD, do not use scanf without specifying a length first
     char file[10];
     fscanf(file, "%s", buf2);
 
+    // GOOD, with 'sscanf' the input can be checked first and enough room allocated [FALSE POSITIVE]
+    if (argc >= 1)
+    {
+		char *src = argv[0];
+		char *dest = (char *)malloc(strlen(src) + 1);
+
+		sscanf(src, "%s", dest);
+	}
+
     return 0;
 }

cpp/ql/test/experimental/query-tests/Security/CWE/semmle/tests/MemoryUnsafeFunctionScan.expected

@@ -1,2 +1,3 @@
-| MemoryUnsafeFunctionScan.cpp:14:5:14:9 | call to scanf | Dangerous use of one of the scanf functions |
-| MemoryUnsafeFunctionScan.cpp:22:5:22:10 | call to fscanf | Dangerous use of one of the scanf functions |
+| MemoryUnsafeFunctionScan.cpp:19:5:19:9 | call to scanf | Dangerous use of one of the scanf functions |
+| MemoryUnsafeFunctionScan.cpp:28:5:28:10 | call to fscanf | Dangerous use of one of the scanf functions |
+| MemoryUnsafeFunctionScan.cpp:36:3:36:8 | call to sscanf | Dangerous use of one of the scanf functions |