Merge branch 'main' into jcogs33/add-toByteArray-summaries

Author: jcogs33

cpp/ql/lib/semmle/code/cpp/models/implementations/StdString.qll

@@ -66,7 +66,7 @@ abstract private class StdStringTaintFunction extends TaintFunction {
    * Gets the index of a parameter to this function that is an iterator.
    */
   final int getAnIteratorParameterIndex() {
-    this.getParameter(result).getType() instanceof Iterator
+    this.getParameter(result).getUnspecifiedType() instanceof Iterator
   }
 }
 

cpp/ql/src/Likely Bugs/Format/WrongTypeFormatArguments.ql

@@ -172,5 +172,5 @@ where
   not arg.isFromUninstantiatedTemplate(_) and
   not actual.getUnspecifiedType() instanceof ErroneousType
 select arg,
-  "This argument should be of type '" + expected.getName() + "' but is of type '" +
+  "This format specifier for type '" + expected.getName() + "' does not match the argument type '" +
     actual.getUnspecifiedType().getName() + "'."

cpp/ql/src/Likely Bugs/Memory Management/ReturnStackAllocatedMemory.cpp

@@ -5,22 +5,23 @@
 
 
 <overview>
-<p>This rule finds return statements that return pointers to an object allocated on the stack. 
-The lifetime of a stack allocated memory location only lasts until the function returns, and 
-the contents of that memory become undefined after that. Clearly, using a pointer to stack 
+<p>This rule finds return statements that return pointers to an object allocated on the stack.
+The lifetime of a stack allocated memory location only lasts until the function returns, and
+the contents of that memory become undefined after that. Clearly, using a pointer to stack
 memory after the function has already returned will have undefined results. </p>
 
 </overview>
 <recommendation>
-<p>Use the functions of the <tt>malloc</tt> family to dynamically allocate memory on the heap for data that is used across function calls.</p>
+<p>Use the functions of the <tt>malloc</tt> family, or <tt>new</tt>, to dynamically allocate memory on the heap for data that is used across function calls.</p>
 
 </recommendation>
-<example><sample src="ReturnStackAllocatedMemory.cpp" />
-
-
-
-
+<example>
+<p>The following example allocates an object on the stack and returns a pointer to it. This is incorrect because the object is deallocated
+when the function returns, and the pointer becomes invalid.</p>
+<sample src="ReturnStackAllocatedMemoryBad.cpp" />
 
+<p>To fix this, allocate the object on the heap using <tt>new</tt> and return a pointer to the heap-allocated object.</p>
+<sample src="ReturnStackAllocatedMemoryGood.cpp" />
 </example>
 
 <references>

cpp/ql/src/Likely Bugs/Memory Management/ReturnStackAllocatedMemory.qhelp

@@ -0,0 +1,5 @@
+Record *mkRecord(int value) {
+	Record myRecord(value);
+
+	return &myRecord; // BAD: returns a pointer to `myRecord`, which is a stack-allocated object.
+}

cpp/ql/src/Likely Bugs/Memory Management/ReturnStackAllocatedMemoryBad.cpp

@@ -0,0 +1,5 @@
+Record *mkRecord(int value) {
+	Record *myRecord = new Record(value);
+
+	return myRecord; // GOOD: returns a pointer to a `myRecord`, which is a heap-allocated object.
+}

cpp/ql/src/Likely Bugs/Memory Management/ReturnStackAllocatedMemoryGood.cpp

@@ -1,5 +1,14 @@
-unsigned limit = get_limit();
-unsigned total = 0;
-while (limit - total > 0) { // wrong: if `total` is greater than `limit` this will underflow and continue executing the loop.
+uint32_t limit = get_limit();
+uint32_t total = 0;
+
+while (limit - total > 0) { // BAD: if `total` is greater than `limit` this will underflow and continue executing the loop.
   total += get_data();
-}
+}
+
+while (total < limit) { // GOOD: never underflows here because there is no arithmetic.
+  total += get_data();
+}
+
+while ((int64_t)limit - total > 0) { // GOOD: never underflows here because the result always fits in an `int64_t`.
+  total += get_data();
+}

cpp/ql/src/Security/CWE/CWE-191/UnsignedDifferenceExpressionComparedZero.c

@@ -1,15 +1,17 @@
 char *file_name;
 FILE *f_ptr;
- 
+
 /* Initialize file_name */
- 
+
 f_ptr = fopen(file_name, "w");
 if (f_ptr == NULL)  {
   /* Handle error */
 }
- 
+
 /* ... */
- 
+
 if (chmod(file_name, S_IRUSR) == -1) {
   /* Handle error */
-}
+}
+
+fclose(f_ptr);

cpp/ql/src/Security/CWE/CWE-367/TOCTOUFilesystemRaceBad.c

@@ -1,8 +1,8 @@
 char *file_name;
 int fd;
- 
+
 /* Initialize file_name */
- 
+
 fd = open(
   file_name,
   O_WRONLY | O_CREAT | O_EXCL,
@@ -11,9 +11,11 @@ fd = open(
 if (fd == -1) {
   /* Handle error */
 }
- 
+
 /* ... */
- 
+
 if (fchmod(fd, S_IRUSR) == -1) {
   /* Handle error */
-}
+}
+
+close(fd);

cpp/ql/src/Security/CWE/CWE-367/TOCTOUFilesystemRaceGood.c

@@ -2,7 +2,7 @@
  * @name Iterator to expired container
  * @description Using an iterator owned by a container whose lifetime has expired may lead to unexpected behavior.
  * @kind problem
- * @precision medium
+ * @precision high
  * @id cpp/iterator-to-expired-container
  * @problem.severity warning
  * @security-severity 8.8