scanf fixes, still need to update qhelp file

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

@@ -1,22 +0,0 @@
-<!DOCTYPE qhelp PUBLIC
-  "-//Semmle//qhelp//EN"
-  "qhelp.dtd">
-<qhelp>
-<overview>
-<p>It is generally considered bad practice to use the scanf, sscanf, and fscanf functions as they are vulnerable to buffer overflows. This may even be the case if a specfied length 
-is provided with "%s", if the the buffer is dynamic in size.
-It is recommended to use the scanf_s, sscanf_s, and fscanf_s functions instead.</p>
-
-</overview>
-
-<recommendation>
-<p>Use the scanf_s, sscanf_s, or fscanf_s functions instead.</p>
-</recommendation>
-
-<references>
-<li>https://cwe.mitre.org/data/definitions/120</li>
-<!--  LocalWords:  CWE
- -->
-</references>
-
-</qhelp>

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

@@ -1,31 +0,0 @@
-/**
- * @name Standard library function that is not memory-safe without a specified length
- * @description Use of a standard library function that is not memory-safe without a specified length.
- * @kind problem
- * @problem.severity warning
- * @precision medium
- * @id cpp/memory-unsafe-function-scan
- * @tags reliability
- *       security
- *       external/cwe/cwe-120
- */
-
-import cpp
-
-predicate memoryUnsafeFunctionParameter(Call c, string message) {
-  exists(string name | c.getTarget().hasGlobalName(name) |
-    (
-      (
-        name = "scanf" or
-        name = "sscanf" or
-        name = "fscanf"
-      )
-    ) and
-    message = "Call to " + name + " is potentially dangerous. Please use " + name + "_s to avoid buffer overflows."
-  )
-}
-
-from FunctionCall call, string message
-where
-memoryUnsafeFunctionParameter(call, message)
-select call, message

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

@@ -0,0 +1,16 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>It is bad practice to use any of the scanf functions without including a specified length within the format parameter, as it will be vulnerable to buffer overflows.</p>
+
+</overview>
+
+<references>
+<li>https://cwe.mitre.org/data/definitions/120</li>
+<!--  LocalWords:  CWE
+ -->
+</references>
+
+</qhelp>

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

@@ -0,0 +1,26 @@
+/**
+ * @name Scanf function without a specified length
+ * @description Use of one of the scanf functions without a specified length.
+ * @kind problem
+ * @problem.severity warning
+ * @precision medium
+ * @id cpp/memory-unsafe-function-scan
+ * @tags reliability
+ *       security
+ *       external/cwe/cwe-120
+ */
+
+import cpp
+import semmle.code.cpp.commons.Scanf
+
+
+from FunctionCall call, ScanfFunction sff
+where
+  call.getTarget() = sff
+  and
+  (
+  call.getArgument(sff.getFormatParameterIndex()).toString().regexpMatch(".*%s.*")
+  or
+  call.getArgument(sff.getFormatParameterIndex()).toString() = (".*%ls.*")
+  )
+select call, "Dangerous use of one of the scanf functions"

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

@@ -8,15 +8,15 @@ int fscanf(const char* str, const char* format, ...);
 
 int main(int argc, char** argv) { 
 
-    // BAD, do not use scanf, use scanf_s instead
+    // BAD, do not use scanf without specifying a length first
     char buf1[10];
     scanf("%s", buf1);
 
-    // BAD, do not use sscanf, use sscanf_s instead
+    // GOOD, length is specified
     char buf2[10];
-    sscanf(buf2, "%s");
+    sscanf(buf2, "%9s");
 
-    // BAD, do not use fscanf, use fscanf_s instead
+    // BAD, do not use scanf without specifying a length first
     char file[10];
     fscanf(file, "%s", buf2);
 

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

@@ -0,0 +1,2 @@
+| MemoryUnsafeFunctionScan.cpp:13:5:13:9 | call to scanf | Dangerous use of one of the scanf functions |
+| MemoryUnsafeFunctionScan.cpp:21:5:21:10 | call to fscanf | Dangerous use of one of the scanf functions |

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

@@ -0,0 +1 @@
+experimental/Security/CWE/CWE-120/MemoryUnsafeFunctionScan.ql

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

@@ -1,11 +0,0 @@
-| MemoryUnsafeFunctionScan.cpp:13:5:13:9 | call to scanf | Call to scanf is potentially dangerous. Please use scanf_s to avoid buffer overflows. |
-| MemoryUnsafeFunctionScan.cpp:17:5:17:10 | call to sscanf | Call to sscanf is potentially dangerous. Please use sscanf_s to avoid buffer overflows. |
-| MemoryUnsafeFunctionScan.cpp:21:5:21:10 | call to fscanf | Call to fscanf is potentially dangerous. Please use fscanf_s to avoid buffer overflows. |
-| tests.c:31:3:31:7 | call to scanf | Call to scanf is potentially dangerous. Please use scanf_s to avoid buffer overflows. |
-| tests.c:32:3:32:7 | call to scanf | Call to scanf is potentially dangerous. Please use scanf_s to avoid buffer overflows. |
-| tests.c:33:3:33:7 | call to scanf | Call to scanf is potentially dangerous. Please use scanf_s to avoid buffer overflows. |
-| tests.c:34:3:34:8 | call to sscanf | Call to sscanf is potentially dangerous. Please use sscanf_s to avoid buffer overflows. |
-| tests.c:60:3:60:7 | call to scanf | Call to scanf is potentially dangerous. Please use scanf_s to avoid buffer overflows. |
-| tests.c:61:3:61:7 | call to scanf | Call to scanf is potentially dangerous. Please use scanf_s to avoid buffer overflows. |
-| tests.c:62:3:62:7 | call to scanf | Call to scanf is potentially dangerous. Please use scanf_s to avoid buffer overflows. |
-| tests.c:63:3:63:7 | call to scanf | Call to scanf is potentially dangerous. Please use scanf_s to avoid buffer overflows. |

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

@@ -1 +0,0 @@
-Security/CWE/CWE-120/MemoryUnsafeFunctionScan.ql