Merge pull request #1141 from geoffw0/newfreebug

CPP: Fix a bug in NewFree.qll

Author: jbj

change-notes/1.21/analysis-cpp.md

@@ -0,0 +1,16 @@
+# Improvements to C/C++ analysis
+
+## General improvements
+
+## New queries
+
+| **Query**                   | **Tags**  | **Purpose**                                                        |
+|-----------------------------|-----------|--------------------------------------------------------------------|
+
+## Changes to existing queries
+
+| **Query**                  | **Expected impact**    | **Change**                                                       |
+|----------------------------|------------------------|------------------------------------------------------------------|
+| Mismatching new/free or malloc/delete (`cpp/new-free-mismatch`) | Fewer false positive results | Fixed an issue where functions were being identified as allocation functions inappropriately.  Also affects `cpp/new-array-delete-mismatch` and `cpp/new-delete-array-mismatch`. |
+
+## Changes to QL libraries

cpp/ql/src/Critical/NewDelete.qll

@@ -3,6 +3,7 @@
  */
 import cpp
 import semmle.code.cpp.controlflow.SSA
+import semmle.code.cpp.dataflow.DataFlow
 
 /**
  * Holds if `alloc` is a use of `malloc` or `new`.  `kind` is
@@ -46,7 +47,10 @@ predicate allocExprOrIndirect(Expr alloc, string kind) {
     alloc.(FunctionCall).getTarget() = rtn.getEnclosingFunction() and
     (
       allocExprOrIndirect(rtn.getExpr(), kind) or
-      allocReaches0(rtn.getExpr(), _, kind)
+      exists(Expr e |
+        allocExprOrIndirect(e, kind) and
+        DataFlow::localFlow(DataFlow::exprNode(e), DataFlow::exprNode(rtn.getExpr()))
+      )
     )
   )
 }

cpp/ql/test/query-tests/Critical/NewFree/test.cpp

@@ -371,3 +371,56 @@ void test12(bool cond)
 		free(z); // GOOD
 	}
 }
+
+// ---
+
+class MyBuffer13
+{
+public:
+	MyBuffer13(int size)
+	{
+		buffer = (char *)malloc(size * sizeof(char));
+	}
+
+	~MyBuffer13()
+	{
+		free(buffer); // GOOD
+	}
+
+	char *getBuffer() // note: this should not be considered an allocation function
+	{
+		return buffer;
+	}
+
+private:
+	char *buffer;
+};
+
+class MyPointer13
+{
+public:
+	MyPointer13(char *_pointer) : pointer(_pointer)
+	{
+	}
+
+	MyPointer13(MyBuffer13 &buffer) : pointer(buffer.getBuffer())
+	{
+	}
+
+	char *getPointer() // note: this should not be considered an allocation function
+	{
+		return pointer;
+	}
+
+private:
+	char *pointer;
+};
+
+void test13()
+{
+	MyBuffer13 myBuffer(100);
+	MyPointer13 myPointer2(myBuffer);
+	MyPointer13 myPointer3(new char[100]);
+
+	delete myPointer3.getPointer(); // GOOD
+}