Skip to content

Commit f074564

Browse files
ihsinmeMathiasVP
andauthored
Apply suggestions from code review
Co-authored-by: Mathias Vorreiter Pedersen <mathiasvp@github.com>
1 parent fcd5325 commit f074564

File tree

3 files changed

+6
-9
lines changed

3 files changed

+6
-9
lines changed
Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11

2-
strncat(dest, src, sizeof(dest) - strlen(dest)); //bad:
2+
strncat(dest, source, sizeof(dest) - strlen(dest)); // BAD: writes a zero byte past the `dest` buffer.
33

4-
strncat(dest, src, sizeof(dest) - strlen(dest) -1); //good:
4+
strncat(dest, source, sizeof(dest) - strlen(dest) -1); // GOOD: Reserves space for the zero byte.

cpp/ql/src/experimental/Security/CWE/CWE-788/AccessOfMemoryLocationAfterEndOfBufferUsingStrncat.qhelp

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,18 +3,17 @@
33
"qhelp.dtd">
44
<qhelp>
55
<overview>
6-
<p>The standard library function <code> strncat </code> appends the source string to the target string. The third argument specifies the maximum number of characters to add and must be less than the remaining space in the target buffer. Calls of the form <code> strncat (dest, src, sizeof (dest) - strlen (dest)) </code> set the third argument to one more than possible. So when the buffer is full, the expression <code> sizeof (dest) - strlen (dest) </code> 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 buffer.</p>
6+
<p>The standard library function <code>strncat(dest, source, count)</code> appends the <code>source</code> string to the <code>dest</code> string. <code>count</code> specifies the maximum number of characters to append and must be less than the remaining space in the target buffer. Calls of the form <code> strncat (dest, source, sizeof (dest) - strlen (dest)) </code> set the third argument to one more than possible. So when the <code>dest</code> is full, the expression <code> sizeof (dest) - strlen (dest) </code> 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 <code>dest</code> buffer.</p>
77

8-
<p>Loss of detection includes cases of use of situations when memory allocation for a buffer occurs with a strong nesting or outside the limits of the function. It is also worth paying attention to the exclusion from the detection of situations when the second argument of the function is a constant string, since this situation creates a large number of false detection.</p>
98

109
</overview>
1110
<recommendation>
1211

13-
<p>We recommend using an extra byte call. for example <code> strncat(dest, src, sizeof(dest)-strlen(dest)-1) </code>.</p>
12+
<p>We recommend subtracting one from the third argument. For example, replace <code>strncat(dest, source, sizeof(dest)-strlen(dest))</code> with <code>strncat(dest, source, sizeof(dest)-strlen(dest)-1)</code>.</p>
1413

1514
</recommendation>
1615
<example>
17-
<p>The following example demonstrates an erroneous and corrected use of the strncat function.</p>
16+
<p>The following example demonstrates an erroneous and corrected use of the <code>strncat</code> function.</p>
1817
<sample src="AccessOfMemoryLocationAfterEndOfBufferUsingStrncat.c" />
1918

2019
</example>

cpp/ql/src/experimental/Security/CWE/CWE-788/AccessOfMemoryLocationAfterEndOfBufferUsingStrncat.ql

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
/**
22
* @name Access Of Memory Location After The End Of A Buffer Using Strncat
3-
* @description --Calls of the form strncat(dest, src, sizeof (dest) - strlen (dest)) set the third argument to one more than possible.
4-
* --So when the buffer is full, the expression sizeof(dest) - strlen (dest) will be equal to one, and not zero as the programmer might think.
5-
* --Making a call of this type may result in a zero byte being written just outside the buffer.
3+
* @description Calls of the form `strncat(dest, source, sizeof (dest) - strlen (dest))` set the third argument to one more than possible. So when `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.
64
* @kind problem
75
* @id cpp/access-memory-location-after-end-buffer
86
* @problem.severity warning

0 commit comments

Comments
 (0)