diff --git a/cpp/ql/src/Security/CWE/CWE-120/BadlyBoundedWrite.ql b/cpp/ql/src/Security/CWE/CWE-120/BadlyBoundedWrite.ql index 247606c683d..e7dd6a5d8e3 100644 --- a/cpp/ql/src/Security/CWE/CWE-120/BadlyBoundedWrite.ql +++ b/cpp/ql/src/Security/CWE/CWE-120/BadlyBoundedWrite.ql @@ -24,7 +24,7 @@ import semmle.code.cpp.security.BufferWrite from BufferWrite bw, int destSize where bw.hasExplicitLimit() and // has an explicit size limit - destSize = getBufferSize(bw.getDest(), _) and + destSize = max(getBufferSize(bw.getDest(), _)) and bw.getExplicitLimit() > destSize // but it's larger than the destination select bw, "This '" + bw.getBWDesc() + "' operation is limited to " + bw.getExplicitLimit() + diff --git a/cpp/ql/src/change-notes/2023-12-15-badly-bounded-write.md b/cpp/ql/src/change-notes/2023-12-15-badly-bounded-write.md new file mode 100644 index 00000000000..1dd4705754b --- /dev/null +++ b/cpp/ql/src/change-notes/2023-12-15-badly-bounded-write.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* The `cpp/badly-bounded-write` query could report false positives when a pointer was first initialized with a literal and later assigned a dynamically allocated array. These false positives now no longer occur. 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 359da3bd2bc..9abc89c68f1 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,4 +1,2 @@ -| tests2.cpp:59:3:59:10 | call to snprintf | This 'call to snprintf' operation is limited to 13 bytes but the destination is only 2 bytes. | -| tests2.cpp:63:3:63:10 | call to snprintf | This 'call to snprintf' operation is limited to 13 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/tests2.cpp b/cpp/ql/test/query-tests/Security/CWE/CWE-120/semmle/tests/tests2.cpp index 111557e9f05..c492e11f0b8 100644 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-120/semmle/tests/tests2.cpp +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-120/semmle/tests/tests2.cpp @@ -56,9 +56,9 @@ void test3() { dest1 = (char*)malloc(sizeof(src)); if (!dest1) return; - snprintf(dest1, sizeof(src), "%s", src); // GOOD [FALSE POSITIVE] + snprintf(dest1, sizeof(src), "%s", src); // GOOD dest2 = (char*)malloc(3); if (!dest2) return; - snprintf(dest2, sizeof(src), "%s", src); // BAD (but with duplicate alerts) + snprintf(dest2, sizeof(src), "%s", src); // BAD [NOT DETECTED] }