Skip to content

Commit 10b6215

Browse files
authored
C++: add cpp/very-likely-overruning-write help
Also update the help of `cpp/overruning-write`, as the case shown there will actually not be flagged by that query any more.
1 parent b979f02 commit 10b6215

File tree

2 files changed

+9
-9
lines changed

2 files changed

+9
-9
lines changed
Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
1-
void sayHello()
1+
void sayHello(uint32_t userId)
22
{
3-
char buffer[10];
3+
char buffer[18];
44

5-
// BAD: this message overflows the buffer
6-
strcpy(buffer, "Hello, world!");
5+
// BAD: this message overflows the buffer if userId >= 10000
6+
sprintf(buffer, "Hello, user %d!", userId);
77

88
MessageBox(hWnd, buffer, "New Message", MB_OK);
99
}

cpp/ql/src/Security/CWE/CWE-120/OverrunWrite.qhelp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,13 @@
1313
<example>
1414
<sample src="OverrunWrite.c" />
1515

16-
<p>In this example, the call to <code>strcpy</code> copies a message of 14 characters (including the terminating null) into a buffer with space for just 10 characters. As such, the last four characters overflow the buffer resulting in undefined behavior.</p>
16+
<p>In this example, the call to <code>sprintf</code> writes a message of 14 characters (including the terminating null) plus the length of the string conversion of `userId` into a buffer with space for just 18 characters. As such, if `userId` is greater or equal to `10000`, the last characters overflow the buffer resulting in undefined behavior.</p>
1717

18-
<p>To fix this issue three changes should be made:</p>
18+
<p>To fix this issue one of three changes should be made:</p>
1919
<ul>
20-
<li>Control the size of the buffer using a preprocessor define.</li>
21-
<li>Replace the call to <code>strcpy</code> with <code>strncpy</code>, specifying the define as the maximum length to copy. This will prevent the buffer overflow.</li>
22-
<li>Consider increasing the buffer size, say to 20 characters, so that the message is displayed correctly.</li>
20+
<li>Preferably, replace the call to <code>sprintf</code> with <code>snprintf</code>, specifying a define or `sizeof(buffer)` as maximum length to copy. This will prevent the buffer overflow.</li>
21+
<li>If `userId` is expected to be less than `10000`, then return or throw an error if `userId` is out of bounds.</li>
22+
<li>Consider increasing the buffer size to at least 25 characters, so that the message is displayed correctly regardless of the value of `userId`.</li>
2323
</ul>
2424

2525
</example>

0 commit comments

Comments
 (0)