Merge pull request #16939 from geoffw0/docsforautofix

C++: Assorted minor doc improvements

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.qhelp

@@ -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/ReturnStackAllocatedMemoryBad.cpp

@@ -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/ReturnStackAllocatedMemoryGood.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/Security/CWE/CWE-191/UnsignedDifferenceExpressionComparedZero.c

@@ -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-367/TOCTOUFilesystemRaceBad.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/TOCTOUFilesystemRaceGood.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-570/IncorrectAllocationErrorHandling.cpp

@@ -34,7 +34,7 @@ void good1(std::size_t length) noexcept {
 
 // GOOD: the allocation failure is handled appropriately.
 void good2(std::size_t length) noexcept {
-  int* dest = new int[length];
+  int* dest = new(std::nothrow) int[length];
   if(!dest) {
     return;
   }

cpp/ql/src/Security/CWE/CWE-732/DoNotCreateWorldWritable.c

@@ -1,11 +1,38 @@
 void write_default_config_bad() {
 	// BAD - this is world-writable so any user can overwrite the config
 	int out = creat(OUTFILE, 0666);
-	dprintf(out, DEFAULT_CONFIG);
+	if (out < 0) {
+		// handle error
+	}
+
+	dprintf(out, "%s", DEFAULT_CONFIG);
+	close(out);
 }
 
 void write_default_config_good() {
 	// GOOD - this allows only the current user to modify the file
 	int out = creat(OUTFILE, S_IWUSR | S_IRUSR);
-	dprintf(out, DEFAULT_CONFIG);
+	if (out < 0) {
+		// handle error
+	}
+
+	dprintf(out, "%s", DEFAULT_CONFIG);
+	close(out);
+}
+
+void write_default_config_good_2() {
+	// GOOD - this allows only the current user to modify the file
+	int out = open(OUTFILE, O_WRONLY | O_CREAT, S_IWUSR | S_IRUSR);
+	if (out < 0) {
+		// handle error
+	}
+
+	FILE *fd = fdopen(out, "w");
+	if (fd == NULL) {
+		close(out);
+		// handle error
+	}
+
+	fprintf(fd, "%s", DEFAULT_CONFIG);
+	fclose(fd);
 }

cpp/ql/src/Security/CWE/CWE-732/DoNotCreateWorldWritable.qhelp

@@ -29,10 +29,11 @@ so it is important that they cannot be controlled by an attacker.
 </p>
 
 <p>
-The first example creates the default configuration file with the usual "default" Unix permissions, <code>0666</code>. This makes the 
+The first example creates the default configuration file with the usual "default" Unix permissions, <code>0666</code>. This makes the
 file world-writable, so that an attacker could write in their own configuration that would be read by the program. The second example uses
 more restrictive permissions: a combination of the standard Unix constants <code>S_IWUSR</code> and <code>S_IRUSR</code> which means that
-only the current user will have read and write access to the file.
+only the current user will have read and write access to the file. The third example shows another way to create a file with more restrictive
+permissions if a <code>FILE *</code> stream pointer is required rather than a file descriptor.
 </p>
 
 <sample src="DoNotCreateWorldWritable.c" />

cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/WrongTypeFormatArguments.expected

@@ -1,12 +1,12 @@
-| tests.cpp:18:15:18:22 | Hello | This argument should be of type 'char *' but is of type 'char16_t *'. |
-| tests.cpp:19:15:19:22 | Hello | This argument should be of type 'char *' but is of type 'wchar_t *'. |
-| tests.cpp:21:15:21:21 | Hello | This argument should be of type 'char16_t *' but is of type 'char *'. |
-| tests.cpp:21:15:21:21 | Hello | This argument should be of type 'wchar_t *' but is of type 'char *'. |
-| tests.cpp:26:17:26:24 | Hello | This argument should be of type 'char *' but is of type 'char16_t *'. |
-| tests.cpp:30:17:30:24 | Hello | This argument should be of type 'wchar_t *' but is of type 'char16_t *'. |
-| tests.cpp:35:36:35:43 | Hello | This argument should be of type 'char *' but is of type 'wchar_t *'. |
-| tests.cpp:39:36:39:43 | Hello | This argument should be of type 'char16_t *' but is of type 'wchar_t *'. |
-| tests.cpp:42:37:42:44 | Hello | This argument should be of type 'char *' but is of type 'char16_t *'. |
-| tests.cpp:43:37:43:44 | Hello | This argument should be of type 'char *' but is of type 'wchar_t *'. |
-| tests.cpp:45:37:45:43 | Hello | This argument should be of type 'char16_t *' but is of type 'char *'. |
-| tests.cpp:47:37:47:44 | Hello | This argument should be of type 'char16_t *' but is of type 'wchar_t *'. |
+| tests.cpp:18:15:18:22 | Hello | This format specifier for type 'char *' does not match the argument type 'char16_t *'. |
+| tests.cpp:19:15:19:22 | Hello | This format specifier for type 'char *' does not match the argument type 'wchar_t *'. |
+| tests.cpp:21:15:21:21 | Hello | This format specifier for type 'char16_t *' does not match the argument type 'char *'. |
+| tests.cpp:21:15:21:21 | Hello | This format specifier for type 'wchar_t *' does not match the argument type 'char *'. |
+| tests.cpp:26:17:26:24 | Hello | This format specifier for type 'char *' does not match the argument type 'char16_t *'. |
+| tests.cpp:30:17:30:24 | Hello | This format specifier for type 'wchar_t *' does not match the argument type 'char16_t *'. |
+| tests.cpp:35:36:35:43 | Hello | This format specifier for type 'char *' does not match the argument type 'wchar_t *'. |
+| tests.cpp:39:36:39:43 | Hello | This format specifier for type 'char16_t *' does not match the argument type 'wchar_t *'. |
+| tests.cpp:42:37:42:44 | Hello | This format specifier for type 'char *' does not match the argument type 'char16_t *'. |
+| tests.cpp:43:37:43:44 | Hello | This format specifier for type 'char *' does not match the argument type 'wchar_t *'. |
+| tests.cpp:45:37:45:43 | Hello | This format specifier for type 'char16_t *' does not match the argument type 'char *'. |
+| tests.cpp:47:37:47:44 | Hello | This format specifier for type 'char16_t *' does not match the argument type 'wchar_t *'. |

cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_word_size/WrongTypeFormatArguments.expected

@@ -1,4 +1,4 @@
-| tests_32.cpp:14:16:14:23 | void_ptr | This argument should be of type 'long' but is of type 'void *'. |
-| tests_32.cpp:15:15:15:15 | l | This argument should be of type 'void *' but is of type 'long'. |
-| tests_64.cpp:14:16:14:23 | void_ptr | This argument should be of type 'long' but is of type 'void *'. |
-| tests_64.cpp:15:15:15:15 | l | This argument should be of type 'void *' but is of type 'long'. |
+| tests_32.cpp:14:16:14:23 | void_ptr | This format specifier for type 'long' does not match the argument type 'void *'. |
+| tests_32.cpp:15:15:15:15 | l | This format specifier for type 'void *' does not match the argument type 'long'. |
+| tests_64.cpp:14:16:14:23 | void_ptr | This format specifier for type 'long' does not match the argument type 'void *'. |
+| tests_64.cpp:15:15:15:15 | l | This format specifier for type 'void *' does not match the argument type 'long'. |