This rule finds calls to a function that ignore the return value. A function call is only marked
-as a violation if at least 80% of the total calls to that function check the return value. Not
+as a violation if at least 90% of the total calls to that function check the return value. Not
checking a return value is a common source of defects from standard library functions like The standard library function malloc or fread.
These functions return the status information and the return values should always be checked
to see if the operation succeeded before operating on any data modified or resources allocated by these functions.
diff --git a/cpp/ql/src/Critical/ReturnValueIgnored.ql b/cpp/ql/src/Critical/ReturnValueIgnored.ql
index b9143085720..b4a4a044068 100644
--- a/cpp/ql/src/Critical/ReturnValueIgnored.ql
+++ b/cpp/ql/src/Critical/ReturnValueIgnored.ql
@@ -1,6 +1,6 @@
/**
* @name Return value of a function is ignored
- * @description A call to a function ignores its return value, but more than 80% of the total number of calls to the function check the return value. Check the return value of functions consistently, especially for functions like 'fread' or the 'scanf' functions that return the status of the operation.
+ * @description A call to a function ignores its return value, but at least 90% of the total number of calls to the function check the return value. Check the return value of functions consistently, especially for functions like 'fread' or the 'scanf' functions that return the status of the operation.
* @kind problem
* @id cpp/return-value-ignored
* @problem.severity recommendation
diff --git a/cpp/ql/src/Likely Bugs/Memory Management/SuspiciousCallToStrncat.cpp b/cpp/ql/src/Likely Bugs/Memory Management/SuspiciousCallToStrncat.cpp
index c5cbcd2d7f1..d15a123ce66 100644
--- a/cpp/ql/src/Likely Bugs/Memory Management/SuspiciousCallToStrncat.cpp
+++ b/cpp/ql/src/Likely Bugs/Memory Management/SuspiciousCallToStrncat.cpp
@@ -2,3 +2,7 @@ strncat(dest, src, strlen(dest)); //wrong: should use remaining size of dest
strncat(dest, src, sizeof(dest)); //wrong: should use remaining size of dest.
//Also fails if dest is a pointer and not an array.
+
+strncat(dest, source, sizeof(dest) - strlen(dest)); // wrong: writes a zero byte past the `dest` buffer.
+
+strncat(dest, source, sizeof(dest) - strlen(dest) - 1); // correct: reserves space for the zero byte.
diff --git a/cpp/ql/src/Likely Bugs/Memory Management/SuspiciousCallToStrncat.qhelp b/cpp/ql/src/Likely Bugs/Memory Management/SuspiciousCallToStrncat.qhelp
index 5424338e1d1..13c1e6d2710 100644
--- a/cpp/ql/src/Likely Bugs/Memory Management/SuspiciousCallToStrncat.qhelp
+++ b/cpp/ql/src/Likely Bugs/Memory Management/SuspiciousCallToStrncat.qhelp
@@ -4,7 +4,17 @@
strncat appends a source string to a target string.
-The third argument defines the maximum number of characters to append and should be less than or equal to the remaining space in the destination buffer. Calls of the form strncat(dest, src, strlen(dest)) or strncat(dest, src, sizeof(dest)) set the third argument to the entire size of the destination buffer. Executing a call of this type may cause a buffer overflow unless the buffer is known to be empty. Buffer overflows can lead to anything from a segmentation fault to a security vulnerability.
Calls of the form strncat(dest, src, strlen(dest)) or strncat(dest, src, sizeof(dest)) set
+the third argument to the entire size of the destination buffer.
+Executing a call of this type may cause a buffer overflow unless the buffer is known to be empty.
Similarly, calls of the form strncat(dest, src, sizeof (dest) - strlen (dest)) allow one
+byte to be written ouside the dest buffer.
Buffer overflows can lead to anything from a segmentation fault to a security vulnerability.
new(std::nothrow)
The code passes user input as part of a SQL query without escaping special elements.
+It generates a SQL query to Postgres using sprintf,
+with the user-supplied data directly passed as an argument
+to sprintf. This leaves the code vulnerable to attack by SQL Injection.
Use a library routine to escape characters in the user-supplied
+string before converting it to SQL. Use esc and quote pqxx library functions.
Using variables with the same name is dangerous. However, such a situation inside the while loop can create an infinite loop exhausting resources. Requires the attention of developers.
+ +We recommend not to use local variables inside a loop if their names are the same as the variables in the condition of this loop.
+ +The following example demonstrates an erroneous and corrected use of a local variable within a loop.
+Freeing a previously allocated resource twice can lead to various vulnerabilities in the program.
+ +We recommend that you exclude situations of possible double release. For example, use the assignment NULL to a freed variable.
+ +The following example demonstrates an erroneous and corrected use of freeing a pointer.
+The standard library function strncat(dest, source, count) appends the source string to the dest string. count specifies the maximum number of characters to append and must be less than the remaining space in the target buffer. Calls of the form strncat (dest, source, sizeof (dest) - strlen (dest)) set the third argument to one more than possible. So when the dest is full, the expression sizeof (dest) - strlen (dest) will be equal to one, and not zero as the programmer might think. Making a call of this type may result in a zero byte being written just outside the dest buffer.
We recommend subtracting one from the third argument. For example, replace strncat(dest, source, sizeof(dest)-strlen(dest)) with strncat(dest, source, sizeof(dest)-strlen(dest)-1).
The following example demonstrates an erroneous and corrected use of the strncat function.