Merge pull request #5766 from ihsinme/ihsinme-patch-267

CPP: Add query for CWE-415 Double Free

cpp/ql/src/experimental/Security/CWE/CWE-415/DoubleFree.c

@@ -0,0 +1,14 @@
+...
+  buf = malloc(intSize);
+...
+  free(buf); 
+  buf = NULL;
+  if(buf) free(buf); // GOOD
+...
+
+...
+  buf = malloc(intSize);
+...
+  free(buf); 
+  if(buf) free(buf); // BAD: the cleanup function does not zero out the pointer
+...

cpp/ql/src/experimental/Security/CWE/CWE-415/DoubleFree.qhelp

@@ -0,0 +1,26 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>Freeing a previously allocated resource twice can lead to various vulnerabilities in the program.</p>
+
+</overview>
+<recommendation>
+<p>We recommend that you exclude situations of possible double release. For example, use the assignment NULL to a freed variable.</p>
+
+</recommendation>
+<example>
+<p>The following example demonstrates an erroneous and corrected use of freeing a pointer.</p>
+<sample src="DoubleFree.c" />
+
+</example>
+<references>
+
+<li>
+  CERT C Coding Standard:
+  <a href="https://wiki.sei.cmu.edu/confluence/display/c/MEM30-C.+Do+not+access+freed+memory">MEM30-C. Do not access freed memory</a>.
+</li>
+
+</references>
+</qhelp>

cpp/ql/src/experimental/Security/CWE/CWE-415/DoubleFree.ql

@@ -0,0 +1,43 @@
+/**
+ * @name Errors When Double Free
+ * @description Freeing a previously allocated resource twice can lead to various vulnerabilities in the program.
+ * @kind problem
+ * @id cpp/double-free
+ * @problem.severity warning
+ * @precision medium
+ * @tags security
+ *       external/cwe/cwe-415
+ */
+
+import cpp
+
+from FunctionCall fc, FunctionCall fc2, LocalScopeVariable v
+where
+  freeCall(fc, v.getAnAccess()) and
+  freeCall(fc2, v.getAnAccess()) and
+  fc != fc2 and
+  fc.getASuccessor*() = fc2 and
+  not exists(Expr exptmp |
+    (exptmp = v.getAnAssignedValue() or exptmp.(AddressOfExpr).getOperand() = v.getAnAccess()) and
+    exptmp = fc.getASuccessor*() and
+    exptmp = fc2.getAPredecessor*()
+  ) and
+  not exists(FunctionCall fctmp |
+    not fctmp instanceof DeallocationExpr and
+    fctmp = fc.getASuccessor*() and
+    fctmp = fc2.getAPredecessor*() and
+    fctmp.getAnArgument().(VariableAccess).getTarget() = v
+  ) and
+  (
+    fc.getTarget().hasGlobalOrStdName("realloc") and
+    (
+      not fc.getParent*() instanceof IfStmt and
+      not exists(IfStmt iftmp |
+        iftmp.getCondition().getAChild*().(VariableAccess).getTarget().getAnAssignedValue() = fc
+      )
+    )
+    or
+    not fc.getTarget().hasGlobalOrStdName("realloc")
+  )
+select fc2.getArgument(0),
+  "This pointer may have already been cleared in the line " + fc.getLocation().getStartLine() + "."