From fd7469848ea7b9f9b857e3e9ec6cca627bccc6fc Mon Sep 17 00:00:00 2001 From: Calum Grant Date: Thu, 5 Dec 2024 13:16:59 +0000 Subject: [PATCH 1/5] C++: Test case for cpp/badly-bounded-write --- .../CWE-120/semmle/tests/BadlyBoundedWrite.expected | 1 + .../Security/CWE/CWE-120/semmle/tests/errors.c | 11 +++++++++++ 2 files changed, 12 insertions(+) create mode 100644 cpp/ql/test/query-tests/Security/CWE/CWE-120/semmle/tests/errors.c diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-120/semmle/tests/BadlyBoundedWrite.expected b/cpp/ql/test/query-tests/Security/CWE/CWE-120/semmle/tests/BadlyBoundedWrite.expected index 9abc89c68f17..0f9b0567c228 100644 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-120/semmle/tests/BadlyBoundedWrite.expected +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-120/semmle/tests/BadlyBoundedWrite.expected @@ -1,2 +1,3 @@ +| errors.c:10:5:10:12 | call to swprintf | This 'call to swprintf' operation is limited to 12 bytes but the destination is only 3 bytes. | | tests.c:43:3:43:10 | call to snprintf | This 'call to snprintf' operation is limited to 111 bytes but the destination is only 110 bytes. | | tests.c:46:3:46:10 | call to snprintf | This 'call to snprintf' operation is limited to 111 bytes but the destination is only 110 bytes. | diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-120/semmle/tests/errors.c b/cpp/ql/test/query-tests/Security/CWE/CWE-120/semmle/tests/errors.c new file mode 100644 index 000000000000..a8f509af154d --- /dev/null +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-120/semmle/tests/errors.c @@ -0,0 +1,11 @@ +// semmle-extractor-options: --expect_errors + +typedef unsigned long size_t; +typedef int wchar_t; + +int swprintf(wchar_t *s, size_t n, const wchar_t *format, ...); + +void test_extraction_errors() { + WCHAR buffer[3]; + swprintf(buffer, 3, L"abc"); +} From b7f47f752b0d80608fd803cce6a91afa3f662719 Mon Sep 17 00:00:00 2001 From: Calum Grant Date: Thu, 5 Dec 2024 14:37:19 +0000 Subject: [PATCH 2/5] C++: Remove FPs from cpp/badly-bounded-write --- cpp/ql/src/Security/CWE/CWE-120/BadlyBoundedWrite.ql | 3 ++- .../CWE/CWE-120/semmle/tests/BadlyBoundedWrite.expected | 1 - 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cpp/ql/src/Security/CWE/CWE-120/BadlyBoundedWrite.ql b/cpp/ql/src/Security/CWE/CWE-120/BadlyBoundedWrite.ql index e7dd6a5d8e39..e89ffac906e6 100644 --- a/cpp/ql/src/Security/CWE/CWE-120/BadlyBoundedWrite.ql +++ b/cpp/ql/src/Security/CWE/CWE-120/BadlyBoundedWrite.ql @@ -25,7 +25,8 @@ from BufferWrite bw, int destSize where bw.hasExplicitLimit() and // has an explicit size limit destSize = max(getBufferSize(bw.getDest(), _)) and - bw.getExplicitLimit() > destSize // but it's larger than the destination + bw.getExplicitLimit() > destSize and // but it's larger than the destination + not bw.getDest().getUnderlyingType().stripType() instanceof ErroneousType // destSize may be incorrect select bw, "This '" + bw.getBWDesc() + "' operation is limited to " + bw.getExplicitLimit() + " bytes but the destination is only " + destSize + " bytes." diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-120/semmle/tests/BadlyBoundedWrite.expected b/cpp/ql/test/query-tests/Security/CWE/CWE-120/semmle/tests/BadlyBoundedWrite.expected index 0f9b0567c228..9abc89c68f17 100644 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-120/semmle/tests/BadlyBoundedWrite.expected +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-120/semmle/tests/BadlyBoundedWrite.expected @@ -1,3 +1,2 @@ -| errors.c:10:5:10:12 | call to swprintf | This 'call to swprintf' operation is limited to 12 bytes but the destination is only 3 bytes. | | tests.c:43:3:43:10 | call to snprintf | This 'call to snprintf' operation is limited to 111 bytes but the destination is only 110 bytes. | | tests.c:46:3:46:10 | call to snprintf | This 'call to snprintf' operation is limited to 111 bytes but the destination is only 110 bytes. | From 7aed4c3cbfedcf9add643c1a802282953bcca242 Mon Sep 17 00:00:00 2001 From: Calum Grant Date: Thu, 5 Dec 2024 15:13:38 +0000 Subject: [PATCH 3/5] C++: Change note --- cpp/ql/src/change-notes/2024-12-05-badly-bounded-write.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 cpp/ql/src/change-notes/2024-12-05-badly-bounded-write.md diff --git a/cpp/ql/src/change-notes/2024-12-05-badly-bounded-write.md b/cpp/ql/src/change-notes/2024-12-05-badly-bounded-write.md new file mode 100644 index 000000000000..2004cd08248e --- /dev/null +++ b/cpp/ql/src/change-notes/2024-12-05-badly-bounded-write.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* The "Badly bounded write" query (`cpp/badly-bounded-write`) query no longer produces results if there is an extraction error in the type of the output buffer. From d38975bb99b746bd6fd27e947279ad003d75d147 Mon Sep 17 00:00:00 2001 From: Calum Grant Date: Fri, 6 Dec 2024 13:07:58 +0000 Subject: [PATCH 4/5] C++: Use getType() instead of getUnderlyingType() --- cpp/ql/src/Security/CWE/CWE-120/BadlyBoundedWrite.ql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/ql/src/Security/CWE/CWE-120/BadlyBoundedWrite.ql b/cpp/ql/src/Security/CWE/CWE-120/BadlyBoundedWrite.ql index e89ffac906e6..69e6e675aa0d 100644 --- a/cpp/ql/src/Security/CWE/CWE-120/BadlyBoundedWrite.ql +++ b/cpp/ql/src/Security/CWE/CWE-120/BadlyBoundedWrite.ql @@ -26,7 +26,7 @@ where bw.hasExplicitLimit() and // has an explicit size limit destSize = max(getBufferSize(bw.getDest(), _)) and bw.getExplicitLimit() > destSize and // but it's larger than the destination - not bw.getDest().getUnderlyingType().stripType() instanceof ErroneousType // destSize may be incorrect + not bw.getDest().getType().stripType() instanceof ErroneousType // destSize may be incorrect select bw, "This '" + bw.getBWDesc() + "' operation is limited to " + bw.getExplicitLimit() + " bytes but the destination is only " + destSize + " bytes." From e98129c402b2250b0cd0dfa0a08ab8124655a67c Mon Sep 17 00:00:00 2001 From: Calum Grant <42069085+calumgrant@users.noreply.github.com> Date: Fri, 6 Dec 2024 15:36:24 +0000 Subject: [PATCH 5/5] Update cpp/ql/src/change-notes/2024-12-05-badly-bounded-write.md Co-authored-by: Jeroen Ketema <93738568+jketema@users.noreply.github.com> --- cpp/ql/src/change-notes/2024-12-05-badly-bounded-write.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/ql/src/change-notes/2024-12-05-badly-bounded-write.md b/cpp/ql/src/change-notes/2024-12-05-badly-bounded-write.md index 2004cd08248e..c7ddd104ad0e 100644 --- a/cpp/ql/src/change-notes/2024-12-05-badly-bounded-write.md +++ b/cpp/ql/src/change-notes/2024-12-05-badly-bounded-write.md @@ -1,4 +1,4 @@ --- category: minorAnalysis --- -* The "Badly bounded write" query (`cpp/badly-bounded-write`) query no longer produces results if there is an extraction error in the type of the output buffer. +* The "Badly bounded write" query (`cpp/badly-bounded-write`) no longer produces results if there is an extraction error in the type of the output buffer.