C++: Promote memset query out of experimental.

Author: MathiasVP

cpp/ql/src/Security/CWE/CWE-014/MemsetMayBeDeleted-bad.c

@@ -0,0 +1,4 @@
+char * password = malloc(PASSWORD_SIZE);
+// ... read and check password
+memset(password, 0, PASSWORD_SIZE);
+free(password);

cpp/ql/src/Security/CWE/CWE-014/MemsetMayBeDeleted-good.c

@@ -0,0 +1,4 @@
+char * password = malloc(PASSWORD_SIZE);
+// ... read and check password
+memset_s(password, PASSWORD_SIZE, 0, PASSWORD_SIZE);
+free(password);

cpp/ql/src/Security/CWE/CWE-014/MemsetMayBeDeleted.qhelp

@@ -0,0 +1,41 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>Calling <code>memset</code> or <code>bzero</code> on a buffer to clear its contents may get optimized
+away by the compiler if the buffer is not subsequently used. This is not desirable behavior if the buffer
+contains sensitive data that could somehow be retrieved by an attacker.</p>
+
+</overview>
+<recommendation>
+
+<p>Use alternative platform-supplied functions that will not get optimized away. Examples of such
+functions include <code>memset_s</code>, <code>SecureZeroMemory</code>, and <code>bzero_explicit</code>.
+Alternatively, passing the <code>-fno-builtin-memset</code> option to the GCC/Clang compiler usually
+also prevents the optimization. Finally, the public-domain <code>secure_memzero</code> function (see
+below) can be used. This function, however, is not guaranteed to work on all platforms and compilers.</p>
+
+</recommendation>
+<example>
+<p>The following program fragment uses <code>memset</code> to erase sensitive information after it is no
+longer needed:</p>
+<sample src="MemsetMayBeDeleted-bad.c" />
+<p>Because of dead store elimination, the call to <code>memset</code> may be removed by the compiler
+(since the buffer is not subsequently used), resulting in potentially sensitive data remaining in memory.
+</p>
+
+<p>The best solution to this problem is to use the <code>memset_s</code> function instead of
+<code>memset</code>:</p>
+<sample src="MemsetMayBeDeleted-good.c" />
+
+</example>
+<references>
+
+<li>
+CERT C Coding Standard:
+<a href="https://wiki.sei.cmu.edu/confluence/display/c/MSC06-C.+Beware+of+compiler+optimizations">MSC06-C. Beware of compiler optimizations</a>.
+</li>
+
+</references>
+</qhelp>

cpp/ql/src/Security/CWE/CWE-014/MemsetMayBeDeleted.ql

@@ -0,0 +1,49 @@
+/**
+ * @name Call to `memset` may be deleted
+ * @description Using <code>memset</code> the function to clear private data in a variable that has no subsequent use
+ *              is potentially dangerous because the compiler can remove the call.
+ * @kind problem
+ * @id cpp/memset-may-be-deleted
+ * @problem.severity warning
+ * @precision high
+ * @tags security
+ *       external/cwe/cwe-14
+ */
+
+import cpp
+import semmle.code.cpp.dataflow.EscapesTree
+import semmle.code.cpp.commons.Exclusions
+
+class MemsetFunction extends Function {
+  MemsetFunction() {
+    this.hasGlobalOrStdOrBslName("memset")
+    or
+    this.hasGlobalOrStdName("wmemset")
+    or
+    this.hasGlobalName(["bzero", "__builtin_memset"])
+  }
+}
+
+from FunctionCall call, LocalVariable v, MemsetFunction memset
+where
+  call.getTarget() = memset and
+  not isFromMacroDefinition(call) and
+  // `v` only escapes as the argument to `memset`.
+  forall(Expr escape | variableAddressEscapesTree(v.getAnAccess(), escape) |
+    call.getArgument(0) = escape.getUnconverted()
+  ) and
+  // `v` is a stack-allocated array or a struct.
+  (
+    v.getUnspecifiedType() instanceof ArrayType and call.getArgument(0) = v.getAnAccess()
+    or
+    v.getUnspecifiedType() instanceof Struct and
+    call.getArgument(0).(AddressOfExpr).getAddressable() = v
+  ) and
+  // There is no later use of `v`.
+  not v.getAnAccess() = call.getASuccessor*() and
+  // Not using the `-fno-builtin-memset` flag
+  exists(Compilation c |
+    c.getAFileCompiled() = call.getFile() and
+    not c.getAnArgument() = "-fno-builtin-memset"
+  )
+select call, "Call to " + memset.getName() + " may be deleted by the compiler."