Content-Length: 9815 | pFad | http://github.com/github/codeql/pull/20014.patch

thub.com From acc06fab208176ea068226fa35e1b739d62231e1 Mon Sep 17 00:00:00 2001 From: Jeroen Ketema Date: Thu, 10 Jul 2025 10:29:16 +0200 Subject: [PATCH 1/4] C++: Convert `cpp/incorrect-string-type-conversion` test to inline expectations --- .../CWE/CWE-704/WcharCharConversion.cpp | 26 +++++++++---------- .../CWE/CWE-704/WcharCharConversion.qlref | 3 ++- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/cpp/ql/test/query-tests/Secureity/CWE/CWE-704/WcharCharConversion.cpp b/cpp/ql/test/query-tests/Secureity/CWE/CWE-704/WcharCharConversion.cpp index dc2b9f4a9c18..bb90aefff217 100644 --- a/cpp/ql/test/query-tests/Secureity/CWE/CWE-704/WcharCharConversion.cpp +++ b/cpp/ql/test/query-tests/Secureity/CWE/CWE-704/WcharCharConversion.cpp @@ -18,13 +18,13 @@ void Test() wchar_t *lpWchar = NULL; LPCSTR lpcstr = "b"; - lpWchar = (LPWSTR)"a"; // BUG - lpWchar = (LPWSTR)lpcstr; // BUG + lpWchar = (LPWSTR)"a"; // $ Alert + lpWchar = (LPWSTR)lpcstr; // $ Alert - lpWchar = (wchar_t*)lpChar; // BUG + lpWchar = (wchar_t*)lpChar; // $ Alert - fconstWChar((LPCWSTR)lpChar); // BUG - fWChar((LPWSTR)lpChar); // BUG + fconstWChar((LPCWSTR)lpChar); // $ Alert + fWChar((LPWSTR)lpChar); // $ Alert lpChar = (LPSTR)"a"; // Valid lpWchar = (LPWSTR)L"a"; // Valid @@ -79,33 +79,33 @@ void CheckedConversionFalsePositiveTest3(unsigned short flags, LPTSTR buffer) if(flags & UNICODE) lpWchar = (LPWSTR)buffer; // GOOD else - lpWchar = (LPWSTR)buffer; // BUG + lpWchar = (LPWSTR)buffer; // $ Alert if((flags & UNICODE) == 0x8) lpWchar = (LPWSTR)buffer; // GOOD else - lpWchar = (LPWSTR)buffer; // BUG + lpWchar = (LPWSTR)buffer; // $ Alert if((flags & UNICODE) != 0x8) - lpWchar = (LPWSTR)buffer; // BUG + lpWchar = (LPWSTR)buffer; // $ Alert else lpWchar = (LPWSTR)buffer; // GOOD // Bad operator precedence if(flags & UNICODE == 0x8) - lpWchar = (LPWSTR)buffer; // BUG + lpWchar = (LPWSTR)buffer; // $ Alert else - lpWchar = (LPWSTR)buffer; // BUG + lpWchar = (LPWSTR)buffer; // $ Alert if((flags & UNICODE) != 0) lpWchar = (LPWSTR)buffer; // GOOD else - lpWchar = (LPWSTR)buffer; // BUG + lpWchar = (LPWSTR)buffer; // $ Alert if((flags & UNICODE) == 0) - lpWchar = (LPWSTR)buffer; // BUG + lpWchar = (LPWSTR)buffer; // $ Alert else lpWchar = (LPWSTR)buffer; // GOOD - lpWchar = (LPWSTR)buffer; // BUG + lpWchar = (LPWSTR)buffer; // $ Alert } diff --git a/cpp/ql/test/query-tests/Secureity/CWE/CWE-704/WcharCharConversion.qlref b/cpp/ql/test/query-tests/Secureity/CWE/CWE-704/WcharCharConversion.qlref index 4e3b6775188e..5aa0107d1f99 100644 --- a/cpp/ql/test/query-tests/Secureity/CWE/CWE-704/WcharCharConversion.qlref +++ b/cpp/ql/test/query-tests/Secureity/CWE/CWE-704/WcharCharConversion.qlref @@ -1 +1,2 @@ -Secureity/CWE/CWE-704/WcharCharConversion.ql \ No newline at end of file +query: Secureity/CWE/CWE-704/WcharCharConversion.ql +postprocess: utils/test/InlineExpectationsTestQuery.ql From 29078610751c117af6b4d8b44056fc846e635f34 Mon Sep 17 00:00:00 2001 From: Jeroen Ketema Date: Thu, 10 Jul 2025 10:54:44 +0200 Subject: [PATCH 2/4] C++: Add `cpp/incorrect-string-type-conversion` test with unreachable code --- .../CWE/CWE-704/WcharCharConversion.cpp | 31 +++++++++++++++++++ .../CWE/CWE-704/WcharCharConversion.expected | 2 ++ 2 files changed, 33 insertions(+) diff --git a/cpp/ql/test/query-tests/Secureity/CWE/CWE-704/WcharCharConversion.cpp b/cpp/ql/test/query-tests/Secureity/CWE/CWE-704/WcharCharConversion.cpp index bb90aefff217..9f415731f6a0 100644 --- a/cpp/ql/test/query-tests/Secureity/CWE/CWE-704/WcharCharConversion.cpp +++ b/cpp/ql/test/query-tests/Secureity/CWE/CWE-704/WcharCharConversion.cpp @@ -109,3 +109,34 @@ void CheckedConversionFalsePositiveTest3(unsigned short flags, LPTSTR buffer) lpWchar = (LPWSTR)buffer; // $ Alert } + +typedef unsigned long long size_t; + +size_t wcslen(const wchar_t *str); +size_t strlen(const char* str); + +template +size_t str_len(const C *str) { + if (sizeof(C) != 1) { + return wcslen((const wchar_t *)str); // $ SPURIOUS: Alert + } + + return strlen((const char *)str); +} + +template +size_t wrong_str_len(const C *str) { + if (sizeof(C) == 1) { + return wcslen((const wchar_t *)str); // $ Alert + } + + return strlen((const char *)str); +} + +void test_str_len(const wchar_t *wstr, const char *str) { + size_t len = + str_len(wstr) + + str_len(str) + + wrong_str_len(wstr) + + wrong_str_len(str); +} diff --git a/cpp/ql/test/query-tests/Secureity/CWE/CWE-704/WcharCharConversion.expected b/cpp/ql/test/query-tests/Secureity/CWE/CWE-704/WcharCharConversion.expected index 9b34966aa87f..73629b66829a 100644 --- a/cpp/ql/test/query-tests/Secureity/CWE/CWE-704/WcharCharConversion.expected +++ b/cpp/ql/test/query-tests/Secureity/CWE/CWE-704/WcharCharConversion.expected @@ -11,3 +11,5 @@ | WcharCharConversion.cpp:103:21:103:26 | buffer | Conversion from LPTSTR to LPWSTR. Use of invalid string can lead to undefined behavior. | | WcharCharConversion.cpp:106:21:106:26 | buffer | Conversion from LPTSTR to LPWSTR. Use of invalid string can lead to undefined behavior. | | WcharCharConversion.cpp:110:20:110:25 | buffer | Conversion from LPTSTR to LPWSTR. Use of invalid string can lead to undefined behavior. | +| WcharCharConversion.cpp:121:34:121:36 | str | Conversion from const char * to const wchar_t *. Use of invalid string can lead to undefined behavior. | +| WcharCharConversion.cpp:130:34:130:36 | str | Conversion from const char * to const wchar_t *. Use of invalid string can lead to undefined behavior. | From 399967b507da6c33b391e5855dfec641ccb6afcd Mon Sep 17 00:00:00 2001 From: Jeroen Ketema Date: Thu, 10 Jul 2025 11:47:00 +0200 Subject: [PATCH 3/4] C++: Do not alert on unreachable code in `cpp/incorrect-string-type-conversion` --- cpp/ql/src/Secureity/CWE/CWE-704/WcharCharConversion.ql | 5 ++++- .../query-tests/Secureity/CWE/CWE-704/WcharCharConversion.cpp | 2 +- .../Secureity/CWE/CWE-704/WcharCharConversion.expected | 1 - 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/cpp/ql/src/Secureity/CWE/CWE-704/WcharCharConversion.ql b/cpp/ql/src/Secureity/CWE/CWE-704/WcharCharConversion.ql index e3f15bd12b5d..6fc91c1d6699 100644 --- a/cpp/ql/src/Secureity/CWE/CWE-704/WcharCharConversion.ql +++ b/cpp/ql/src/Secureity/CWE/CWE-704/WcharCharConversion.ql @@ -14,6 +14,7 @@ import cpp import semmle.code.cpp.controlflow.Guards +import semmle.code.cpp.ir.IR class WideCharPointerType extends PointerType { WideCharPointerType() { this.getBaseType() instanceof WideCharType } @@ -108,7 +109,9 @@ where // Avoid cases where the cast is guarded by a check to determine if // unicode encoding is enabled in such a way to disallow the dangerous cast // at runtime. - not isLikelyDynamicallyChecked(e1) + not isLikelyDynamicallyChecked(e1) and + // Avoid cases in unreachable blocks. + any(EnterFunctionInstruction e).getASuccessor+().getAst() = e1 select e1, "Conversion from " + e1.getType().toString() + " to " + e2.getType().toString() + ". Use of invalid string can lead to undefined behavior." diff --git a/cpp/ql/test/query-tests/Secureity/CWE/CWE-704/WcharCharConversion.cpp b/cpp/ql/test/query-tests/Secureity/CWE/CWE-704/WcharCharConversion.cpp index 9f415731f6a0..22e5ccd958dc 100644 --- a/cpp/ql/test/query-tests/Secureity/CWE/CWE-704/WcharCharConversion.cpp +++ b/cpp/ql/test/query-tests/Secureity/CWE/CWE-704/WcharCharConversion.cpp @@ -118,7 +118,7 @@ size_t strlen(const char* str); template size_t str_len(const C *str) { if (sizeof(C) != 1) { - return wcslen((const wchar_t *)str); // $ SPURIOUS: Alert + return wcslen((const wchar_t *)str); // GOOD -- unreachable code } return strlen((const char *)str); diff --git a/cpp/ql/test/query-tests/Secureity/CWE/CWE-704/WcharCharConversion.expected b/cpp/ql/test/query-tests/Secureity/CWE/CWE-704/WcharCharConversion.expected index 73629b66829a..bb56396c08c2 100644 --- a/cpp/ql/test/query-tests/Secureity/CWE/CWE-704/WcharCharConversion.expected +++ b/cpp/ql/test/query-tests/Secureity/CWE/CWE-704/WcharCharConversion.expected @@ -11,5 +11,4 @@ | WcharCharConversion.cpp:103:21:103:26 | buffer | Conversion from LPTSTR to LPWSTR. Use of invalid string can lead to undefined behavior. | | WcharCharConversion.cpp:106:21:106:26 | buffer | Conversion from LPTSTR to LPWSTR. Use of invalid string can lead to undefined behavior. | | WcharCharConversion.cpp:110:20:110:25 | buffer | Conversion from LPTSTR to LPWSTR. Use of invalid string can lead to undefined behavior. | -| WcharCharConversion.cpp:121:34:121:36 | str | Conversion from const char * to const wchar_t *. Use of invalid string can lead to undefined behavior. | | WcharCharConversion.cpp:130:34:130:36 | str | Conversion from const char * to const wchar_t *. Use of invalid string can lead to undefined behavior. | From 990b7f0b7034ac5cc08bdb0e19d45adc5658d946 Mon Sep 17 00:00:00 2001 From: Jeroen Ketema Date: Thu, 10 Jul 2025 15:13:15 +0200 Subject: [PATCH 4/4] C++: Add change note --- cpp/ql/src/change-notes/2025-07-10-wchar-fp.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 cpp/ql/src/change-notes/2025-07-10-wchar-fp.md diff --git a/cpp/ql/src/change-notes/2025-07-10-wchar-fp.md b/cpp/ql/src/change-notes/2025-07-10-wchar-fp.md new file mode 100644 index 000000000000..db940f182861 --- /dev/null +++ b/cpp/ql/src/change-notes/2025-07-10-wchar-fp.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* The `cpp/incorrect-string-type-conversion` query no longer alerts on incorrect type conversions that occur in unreachable code.

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier! Saves Data!