Merge pull request #5010 from ihsinme/ihsinme-patch-220

CPP: Add query for CWE-570 detect and handle memory allocation errors.

cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.cpp

@@ -0,0 +1,35 @@
+// BAD: on memory allocation error, the program terminates.
+void badFunction(const int *source, std::size_t length) noexcept {
+  int * dest = new int[length];
+  std::memset(dest, 0, length);
+// ..
+}
+// GOOD: memory allocation error will be handled.
+void goodFunction(const int *source, std::size_t length) noexcept {
+  try {
+       int * dest = new int[length];
+  } catch(std::bad_alloc) {
+    // ...
+  }
+  std::memset(dest, 0, length);
+// ..
+}
+// BAD: memory allocation error will not be handled.
+void badFunction(const int *source, std::size_t length) noexcept {
+  try {
+       int * dest = new (std::nothrow) int[length];
+  } catch(std::bad_alloc) {
+    // ...
+  }
+  std::memset(dest, 0, length);
+// ..
+}
+// GOOD: memory allocation error will be handled.
+void goodFunction(const int *source, std::size_t length) noexcept {
+  int * dest = new (std::nothrow) int[length];
+  if (!dest) {
+      return;
+  }
+  std::memset(dest, 0, length);
+// ..
+}

cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.qhelp

@@ -0,0 +1,27 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>When using the <code>new</code> operator to allocate memory, you need to pay attention to the different ways of detecting errors. <code>::operator new(std::size_t)</code> throws an exception on error, whereas <code>::operator new(std::size_t, const std::nothrow_t &amp;)</code> returns zero on error. The programmer can get confused and check the error that occurs when allocating memory incorrectly. That can lead to an unhandled program termination or to a violation of the program logic.</p>
+
+</overview>
+<recommendation>
+
+<p>Use the correct error detection method corresponding with the memory allocation.</p>
+
+</recommendation>
+<example>
+<p>The following example demonstrates various approaches to detecting memory allocation errors using the <code>new</code> operator.</p>
+<sample src="WrongInDetectingAndHandlingMemoryAllocationErrors.cpp" />
+
+</example>
+<references>
+
+<li>
+  CERT C++ Coding Standard:
+<a href="https://wiki.sei.cmu.edu/confluence/display/cplusplus/MEM52-CPP.+Detect+and+handle+memory+allocation+errors">MEM52-CPP. Detect and handle memory allocation errors</a>.
+</li>
+
+</references>
+</qhelp>

cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.ql

@@ -0,0 +1,87 @@
+/**
+ * @name Detect And Handle Memory Allocation Errors
+ * @description --::operator new(std::size_t) throws an exception on error, and ::operator new(std::size_t, const std::nothrow_t &) returns zero on error.
+ *              --the programmer can get confused when check the error that occurs when allocating memory incorrectly.
+ * @kind problem
+ * @id cpp/detect-and-handle-memory-allocation-errors
+ * @problem.severity warning
+ * @precision medium
+ * @tags correctness
+ *       security
+ *       external/cwe/cwe-570
+ */
+
+import cpp
+
+/**
+ * Lookup if condition compare with 0
+ */
+class IfCompareWithZero extends IfStmt {
+  IfCompareWithZero() {
+    this.getCondition().(EQExpr).getAChild().getValue() = "0"
+    or
+    this.getCondition().(NEExpr).getAChild().getValue() = "0" and
+    this.hasElse()
+    or
+    this.getCondition().(NEExpr).getAChild().getValue() = "0" and
+    this.getThen().getAChild*() instanceof ReturnStmt
+  }
+}
+
+/**
+ * lookup for calls to `operator new`, with incorrect error handling.
+ */
+class WrongCheckErrorOperatorNew extends FunctionCall {
+  Expr exp;
+
+  WrongCheckErrorOperatorNew() {
+    this = exp.(NewOrNewArrayExpr).getAChild().(FunctionCall) and
+    (
+      this.getTarget().hasGlobalOrStdName("operator new")
+      or
+      this.getTarget().hasGlobalOrStdName("operator new[]")
+    )
+  }
+
+  /**
+   * Holds if handler `try ... catch` exists.
+   */
+  predicate isExistsTryCatchBlock() {
+    exists(TryStmt ts | this.getEnclosingStmt() = ts.getStmt().getAChild*())
+  }
+
+  /**
+   * Holds if results call `operator new` check in `operator if`.
+   */
+  predicate isExistsIfCondition() {
+    exists(IfCompareWithZero ifc, AssignExpr aex, Initializer it |
+      // call `operator new` directly from the condition of `operator if`.
+      this = ifc.getCondition().getAChild*()
+      or
+      // check results call `operator new` with variable appropriation
+      postDominates(ifc, this) and
+      aex.getAChild() = exp and
+      ifc.getCondition().getAChild().(VariableAccess).getTarget() =
+        aex.getLValue().(VariableAccess).getTarget()
+      or
+      // check results call `operator new` with declaration variable
+      postDominates(ifc, this) and
+      exp = it.getExpr() and
+      it.getDeclaration() = ifc.getCondition().getAChild().(VariableAccess).getTarget()
+    )
+  }
+
+  /**
+   * Holds if `(std::nothrow)` exists in call `operator new`.
+   */
+  predicate isExistsNothrow() { this.getAChild().toString() = "nothrow" }
+}
+
+from WrongCheckErrorOperatorNew op
+where
+  // use call `operator new` with `(std::nothrow)` and checking error using `try ... catch` block and not `operator if`
+  op.isExistsNothrow() and not op.isExistsIfCondition() and op.isExistsTryCatchBlock()
+  or
+  // use call `operator new` without `(std::nothrow)` and checking error using `operator if` and not  `try ... catch` block
+  not op.isExistsNothrow() and not op.isExistsTryCatchBlock() and op.isExistsIfCondition()
+select op, "memory allocation error check is incorrect or missing"

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

@@ -0,0 +1,5 @@
+| test.cpp:30:15:30:26 | call to operator new[] | memory allocation error check is incorrect or missing |
+| test.cpp:38:9:38:20 | call to operator new[] | memory allocation error check is incorrect or missing |
+| test.cpp:50:13:50:38 | call to operator new[] | memory allocation error check is incorrect or missing |
+| test.cpp:51:22:51:47 | call to operator new[] | memory allocation error check is incorrect or missing |
+| test.cpp:53:18:53:43 | call to operator new[] | memory allocation error check is incorrect or missing |

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

@@ -0,0 +1 @@
+experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.ql

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

@@ -0,0 +1,97 @@
+#define NULL ((void*)0)
+class exception {};
+
+namespace std{
+  struct nothrow_t {};
+  typedef unsigned long size_t;
+  class bad_alloc{
+    const char* what() const throw();
+  };
+  extern const std::nothrow_t nothrow;
+}
+
+using namespace std;
+
+void* operator new(std::size_t _Size);
+void* operator new[](std::size_t _Size);
+void* operator new( std::size_t count, const std::nothrow_t& tag );
+void* operator new[]( std::size_t count, const std::nothrow_t& tag );
+
+void badNew_0_0()
+{
+    while (true) {
+        new int[100]; // BAD [NOT DETECTED]
+        if(!(new int[100])) // BAD [NOT DETECTED]
+            return;
+    }
+}
+void badNew_0_1()
+{
+    int * i = new int[100]; // BAD
+    if(i == 0)
+        return;
+    if(!i)
+        return;
+    if(i == NULL)
+        return;
+    int * j;
+    j = new int[100]; // BAD
+    if(j == 0)
+        return;
+    if(!j)
+        return;
+    if(j == NULL)
+        return;
+}
+void badNew_1_0()
+{
+    try {
+        while (true) {
+            new(std::nothrow) int[100]; // BAD
+            int* p = new(std::nothrow) int[100]; // BAD
+            int* p1;
+            p1 = new(std::nothrow) int[100]; // BAD
+        }
+    } catch (const exception &){//const std::bad_alloc& e) {
+//        std::cout << e.what() << '\n';
+    }
+}
+void badNew_1_1()
+{
+    while (true) {
+        int* p = new(std::nothrow) int[100]; // BAD [NOT DETECTED]
+        new(std::nothrow) int[100]; // BAD [NOT DETECTED]
+    }
+}
+
+void goodNew_0_0()
+{
+    try {
+        while (true) {
+            new int[100]; // GOOD
+        }
+    } catch (const exception &){//const std::bad_alloc& e) {
+//        std::cout << e.what() << '\n';
+    }
+}
+
+void goodNew_1_0()
+{
+    while (true) {
+        int* p = new(std::nothrow) int[100]; // GOOD
+        if (p == nullptr) {
+//            std::cout << "Allocation returned nullptr\n";
+            break;
+        }
+        int* p1;
+        p1 = new(std::nothrow) int[100]; // GOOD
+        if (p1 == nullptr) {
+//            std::cout << "Allocation returned nullptr\n";
+            break;
+        }
+        if (new(std::nothrow) int[100] == nullptr) { // GOOD
+//            std::cout << "Allocation returned nullptr\n";
+            break;
+        }
+    }
+}