Merge pull request #5804 from MathiasVP/improve-detect-and-handle-memory-allocation-errors

C++: Improve qhelp and tests for cpp/detect-and-handle-memory-allocation-errors

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

@@ -1,35 +1,43 @@
-// BAD: on memory allocation error, the program terminates.
-void badFunction(const int *source, std::size_t length) noexcept {
-  int * dest = new int[length];
+// BAD: the allocation will throw an unhandled exception
+// instead of returning a null pointer.
+void bad1(std::size_t length) noexcept {
+  int* dest = new int[length];
+  if(!dest) {
+    return;
+  }
   std::memset(dest, 0, length);
-// ..
+  // ...
 }
-// GOOD: memory allocation error will be handled.
-void goodFunction(const int *source, std::size_t length) noexcept {
+
+// BAD: the allocation won't throw an exception, but
+// instead return a null pointer.
+void bad2(std::size_t length) noexcept {
   try {
-       int * dest = new int[length];
-  } catch(std::bad_alloc) {
+    int* dest = new(std::nothrow) int[length];
+    std::memset(dest, 0, 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 {
+
+// GOOD: the allocation failure is handled appropiately.
+void good1(std::size_t length) noexcept {
   try {
-       int * dest = new (std::nothrow) int[length];
-  } catch(std::bad_alloc) {
+    int* dest = new int[length];
+    std::memset(dest, 0, 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;
+
+// GOOD: the allocation failure is handled appropiately.
+void good2(std::size_t length) noexcept {
+  int* dest = new int[length];
+  if(!dest) {
+    return;
   }
   std::memset(dest, 0, length);
-// ..
+  // ...
 }

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

@@ -3,16 +3,20 @@
   "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>
+<p>Different overloads of the <code>new</code> operator handle allocation failures in different ways.
+If <code>new T</code> fails for some type <code>T</code>, it throws a <code>std::bad_alloc</code> exception,
+but <code>new(std::nothrow) T</code> returns a null pointer. If the programmer does not use the corresponding
+method of error handling, allocation failure may go unhandled and could cause the program to behave in
+unexpected ways.</p>
 
 </overview>
 <recommendation>
 
-<p>Use the correct error detection method corresponding with the memory allocation.</p>
+<p>Make sure that exceptions are handled appropriately if <code>new T</code> is used. On the other hand,
+make sure to handle the possibility of null pointers if <code>new(std::nothrow) T</code> is used.</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>

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

@@ -1,7 +1,6 @@
 /**
  * @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.
+ * @description `operator new` throws an exception on allocation failures, while `operator new(std::nothrow)` returns a null pointer. Mixing up these two failure conditions can result in unexpected behavior.
  * @kind problem
  * @id cpp/detect-and-handle-memory-allocation-errors
  * @problem.severity warning