From 06d8892e03008533712bcabfbd94daae4d335567 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Thu, 2 May 2024 14:22:27 +0100 Subject: [PATCH 1/4] C++: Rename an example file. --- .../{StrncpyFlippedArgs.cpp => StrncpyFlippedArgsBad.cpp} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename cpp/ql/src/Likely Bugs/Memory Management/{StrncpyFlippedArgs.cpp => StrncpyFlippedArgsBad.cpp} (100%) diff --git a/cpp/ql/src/Likely Bugs/Memory Management/StrncpyFlippedArgs.cpp b/cpp/ql/src/Likely Bugs/Memory Management/StrncpyFlippedArgsBad.cpp similarity index 100% rename from cpp/ql/src/Likely Bugs/Memory Management/StrncpyFlippedArgs.cpp rename to cpp/ql/src/Likely Bugs/Memory Management/StrncpyFlippedArgsBad.cpp From 8a261b7e7a2ac760daa54dd126f13fcf78290a0a Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Thu, 2 May 2024 14:31:26 +0100 Subject: [PATCH 2/4] C++: Update StrncpyFlippedArgs.qhelp. --- .../Memory Management/StrncpyFlippedArgs.qhelp | 9 +++++++-- .../Memory Management/StrncpyFlippedArgsBad.cpp | 11 +++++++++-- .../Memory Management/StrncpyFlippedArgsGood.cpp | 10 ++++++++++ 3 files changed, 26 insertions(+), 4 deletions(-) create mode 100644 cpp/ql/src/Likely Bugs/Memory Management/StrncpyFlippedArgsGood.cpp diff --git a/cpp/ql/src/Likely Bugs/Memory Management/StrncpyFlippedArgs.qhelp b/cpp/ql/src/Likely Bugs/Memory Management/StrncpyFlippedArgs.qhelp index 2e297116710..9ba2b7c7c8e 100644 --- a/cpp/ql/src/Likely Bugs/Memory Management/StrncpyFlippedArgs.qhelp +++ b/cpp/ql/src/Likely Bugs/Memory Management/StrncpyFlippedArgs.qhelp @@ -3,7 +3,7 @@ "qhelp.dtd"> -

The standard library function strncpy copies a source string to a destination buffer. The third argument defines the maximum number of characters to copy and should be less than +

The standard library function strncpy copies a source string to a destination buffer. The third argument defines the maximum number of characters to copy and should be less than or equal to the size of the destination buffer. Calls of the form strncpy(dest, src, strlen(src)) or strncpy(dest, src, sizeof(src)) incorrectly set the third argument to the size of the source buffer. Executing a call of this type may cause a buffer overflow. Buffer overflows can lead to anything from a segmentation fault to a security vulnerability.

 @@ -12,9 +12,14 @@ or equal to the size of the destination buffer. Calls of the form strncpy( not the source buffer.

- +

In the following examples, the size of the source buffer is incorrectly used as a parameter to strncpy:

+ + +

The corrected version uses the size of the destination buffer, or a variable containing the size of the destination buffer as the size parameter to strncpy:

+ + diff --git a/cpp/ql/src/Likely Bugs/Memory Management/StrncpyFlippedArgsBad.cpp b/cpp/ql/src/Likely Bugs/Memory Management/StrncpyFlippedArgsBad.cpp index 07acc91cd5a..952550b2638 100644 --- a/cpp/ql/src/Likely Bugs/Memory Management/StrncpyFlippedArgsBad.cpp +++ b/cpp/ql/src/Likely Bugs/Memory Management/StrncpyFlippedArgsBad.cpp @@ -1,2 +1,9 @@ -strncpy(dest, src, sizeof(src)); //wrong: size of dest should be used -strncpy(dest, src, strlen(src)); //wrong: size of dest should be used +char src[256]; +char dest1[128]; + +... + +strncpy(dest1, src, sizeof(src)); // wrong: size of dest should be used + +char *dest2 = (char *)malloc(sz1 + sz2 + sz3); +strncpy(dest2, src, strlen(src)); // wrong: size of dest should be used diff --git a/cpp/ql/src/Likely Bugs/Memory Management/StrncpyFlippedArgsGood.cpp b/cpp/ql/src/Likely Bugs/Memory Management/StrncpyFlippedArgsGood.cpp new file mode 100644 index 00000000000..22fc4ebd222 --- /dev/null +++ b/cpp/ql/src/Likely Bugs/Memory Management/StrncpyFlippedArgsGood.cpp @@ -0,0 +1,10 @@ +char src[256]; +char dest1[128]; + +... + +strncpy(dest1, src, sizeof(dest1)); // correct + +size_t destSize = sz1 + sz2 + sz3; +char *dest2 = (char *)malloc(destSize); +strncpy(dest2, src, destSize); // correct From ecbf7aef181553960e87b231c1668655106183c8 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Thu, 2 May 2024 17:26:24 +0100 Subject: [PATCH 3/4] C++: Fix qhelp formatting. --- .../Likely Bugs/Memory Management/StrncpyFlippedArgs.qhelp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/cpp/ql/src/Likely Bugs/Memory Management/StrncpyFlippedArgs.qhelp b/cpp/ql/src/Likely Bugs/Memory Management/StrncpyFlippedArgs.qhelp index 9ba2b7c7c8e..4ef13551ad2 100644 --- a/cpp/ql/src/Likely Bugs/Memory Management/StrncpyFlippedArgs.qhelp +++ b/cpp/ql/src/Likely Bugs/Memory Management/StrncpyFlippedArgs.qhelp @@ -13,15 +13,16 @@ not the source buffer.

+

In the following examples, the size of the source buffer is incorrectly used as a parameter to strncpy:

- +

The corrected version uses the size of the destination buffer, or a variable containing the size of the destination buffer as the size parameter to strncpy:

- - + +
  • cplusplus.com: strncpy.
    • From f5431abb1095ecf3ca9a27d4f7c8ad92bfc47995 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Thu, 2 May 2024 17:37:52 +0100 Subject: [PATCH 4/4] C++: Fix strncpy reference link (the old link was broken). --- .../src/Likely Bugs/Memory Management/StrncpyFlippedArgs.qhelp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/ql/src/Likely Bugs/Memory Management/StrncpyFlippedArgs.qhelp b/cpp/ql/src/Likely Bugs/Memory Management/StrncpyFlippedArgs.qhelp index 4ef13551ad2..201b9057499 100644 --- a/cpp/ql/src/Likely Bugs/Memory Management/StrncpyFlippedArgs.qhelp +++ b/cpp/ql/src/Likely Bugs/Memory Management/StrncpyFlippedArgs.qhelp @@ -25,7 +25,7 @@ not the source buffer.

    -
  • cplusplus.com: strncpy.
    • +
  • cplusplus.com: strncpy.
  • I. Gerg. An Overview and Example of the Buffer-Overflow Exploit. IANewsletter vol 7 no 4. 2005.