From 230724c0851502d8934bf088969c4464f2e93609 Mon Sep 17 00:00:00 2001 From: Raul Garcia Date: Tue, 2 Oct 2018 11:17:23 -0700 Subject: [PATCH] Updates based on feedback --- .gitignore | 1 + .../CWE/CWE-704/WcharCharConversion.cpp | 4 +-- .../CWE/CWE-704/WcharCharConversion.qhelp | 18 ++++++++--- .../CWE/CWE-704/WcharCharConversion.ql | 17 +++-------- .../CWE/CWE-704/WcharCharConversion.cpp | 30 ++++++++----------- .../CWE/CWE-704/WcharCharConversion.expected | 13 ++++---- 6 files changed, 38 insertions(+), 45 deletions(-) diff --git a/.gitignore b/.gitignore index 671c5aefe0b..72882ff86a8 100644 --- a/.gitignore +++ b/.gitignore @@ -14,3 +14,4 @@ /.vs/ProjectSettings.json /.vs/ql/v15/.suo +/.vs/VSWorkspaceState.json diff --git a/cpp/ql/src/Security/CWE/CWE-704/WcharCharConversion.cpp b/cpp/ql/src/Security/CWE/CWE-704/WcharCharConversion.cpp index cca43de59b3..0f4a819cc85 100644 --- a/cpp/ql/src/Security/CWE/CWE-704/WcharCharConversion.cpp +++ b/cpp/ql/src/Security/CWE/CWE-704/WcharCharConversion.cpp @@ -1,3 +1,3 @@ -LPWSTR pSrc; +wchar_t* pSrc; -pSrc = (LPWSTR)"a"; \ No newline at end of file +pSrc = (wchar_t*)"a"; // casting a byte-string literal "a" to a wide-character string \ No newline at end of file diff --git a/cpp/ql/src/Security/CWE/CWE-704/WcharCharConversion.qhelp b/cpp/ql/src/Security/CWE/CWE-704/WcharCharConversion.qhelp index e3656e7101a..270b2141f40 100644 --- a/cpp/ql/src/Security/CWE/CWE-704/WcharCharConversion.qhelp +++ b/cpp/ql/src/Security/CWE/CWE-704/WcharCharConversion.qhelp @@ -4,22 +4,32 @@ -

This rule indicates a potentially incorrect cast from/to an ANSI string (char *) to/from a Unicode string (wchar_t *).

+

This rule indicates a potentially incorrect cast from an byte string (char *) to a wide-character string (wchar_t *).

This cast might yield strings that are not correctly terminated; including potential buffer overruns when using such strings with some dangerous APIs.

 -

Do not explicitly casting ANSI strings to/from Unicode strings.

+

Do not explicitly cast byte strings to wide-character strings.

+

For string literals, prepend the literal string with the letter "L" to indicate that the string is a wide-character string (wchar_t *).

+

For converting a byte literal to a wide-character string literal, you would need to use the appropriate conversion function for the platform you are using. Please see the references section for options according to your platform.

 -

In the following example, an ANSI string literal ("a") is casted as a Unicode string.

+

In the following example, an byte string literal ("a") is cast to a wide-character string.

-

To fix this issue, prepend the literal with the letter "L" (L"a") to define it as a Unicode string.

+

To fix this issue, prepend the literal with the letter "L" (L"a") to define it as a wide-character string.

 +
  • + General resources: + std::mbstowcs +
    • +
  • + Microsoft specific resources: + Security Considerations: International Features +
    • diff --git a/cpp/ql/src/Security/CWE/CWE-704/WcharCharConversion.ql b/cpp/ql/src/Security/CWE/CWE-704/WcharCharConversion.ql index 1178079286b..42687a7b07c 100644 --- a/cpp/ql/src/Security/CWE/CWE-704/WcharCharConversion.ql +++ b/cpp/ql/src/Security/CWE/CWE-704/WcharCharConversion.ql @@ -21,18 +21,9 @@ class WideCharPointerType extends PointerType { from Expr e1, Cast e2 where - e2 = e1.getConversion() - and - ( - exists( WideCharPointerType w, CharPointerType c | - w = e1.getType().getUnspecifiedType().(PointerType) - and c = e2.getType().getUnspecifiedType().(PointerType) - ) - or exists - ( - WideCharPointerType w, CharPointerType c | - w = e2.getType().getUnspecifiedType().(PointerType) - and c = e1.getType().getUnspecifiedType().(PointerType) - ) + e2 = e1.getConversion() and + exists(WideCharPointerType w, CharPointerType c | + w = e2.getType().getUnspecifiedType().(PointerType) and + c = e1.getType().getUnspecifiedType().(PointerType) ) select e1, "Conversion from " + e1.getType().toString() + " to " + e2.getType().toString() + ". Use of invalid string can lead to undefined behavior." \ No newline at end of file diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-704/WcharCharConversion.cpp b/cpp/ql/test/query-tests/Security/CWE/CWE-704/WcharCharConversion.cpp index 9cadc6c8474..d03b26f04e2 100644 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-704/WcharCharConversion.cpp +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-704/WcharCharConversion.cpp @@ -9,32 +9,26 @@ typedef CONST WCHAR *LPCWSTR; typedef CHAR *LPSTR; typedef CONST CHAR *LPCSTR; -void fconstChar(LPCSTR p) {} -void fChar(LPSTR p) {} void fconstWChar(LPCWSTR p) {} void fWChar(LPWSTR p) {} void Test() { - char *lpChar = NULL; - wchar_t *lpWchar = NULL; + char *lpChar = NULL; + wchar_t *lpWchar = NULL; + LPCSTR lpcstr = "b"; - lpChar = (LPSTR)L"a"; // BUG - lpWchar = (LPWSTR)"a"; // BUG + lpWchar = (LPWSTR)"a"; // BUG + lpWchar = (LPWSTR)lpcstr; // BUG - lpChar = (char*)lpWchar; // BUG - lpWchar = (wchar_t*)lpChar; // BUG + lpWchar = (wchar_t*)lpChar; // BUG - fconstChar((LPCSTR)lpWchar); // BUG - fChar((LPSTR)lpWchar); // BUG - fconstWChar((LPCWSTR)lpChar); // BUG - fWChar((LPWSTR)lpChar); // BUG + fconstWChar((LPCWSTR)lpChar); // BUG + fWChar((LPWSTR)lpChar); // BUG - lpChar = (LPSTR)"a"; // Valid - lpWchar = (LPWSTR)L"a"; // Valid + lpChar = (LPSTR)"a"; // Valid + lpWchar = (LPWSTR)L"a"; // Valid - fconstChar((LPCSTR)lpChar); // Valid - fChar(lpChar); // Valid - fconstWChar((LPCWSTR)lpWchar); // Valid - fWChar(lpWchar); // Valid + fconstWChar((LPCWSTR)lpWchar); // Valid + fWChar(lpWchar); // Valid } \ No newline at end of file diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-704/WcharCharConversion.expected b/cpp/ql/test/query-tests/Security/CWE/CWE-704/WcharCharConversion.expected index 8a2af894c39..73787d4f6eb 100644 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-704/WcharCharConversion.expected +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-704/WcharCharConversion.expected @@ -1,8 +1,5 @@ -| WcharCharConversion.cpp:22:18:22:21 | array to pointer conversion | Conversion from const wchar_t * to LPSTR. Use of invalid string can lead to undefined behavior. | -| WcharCharConversion.cpp:23:20:23:22 | array to pointer conversion | Conversion from const char * to LPWSTR. Use of invalid string can lead to undefined behavior. | -| WcharCharConversion.cpp:25:18:25:24 | lpWchar | Conversion from wchar_t * to char *. Use of invalid string can lead to undefined behavior. | -| WcharCharConversion.cpp:26:22:26:27 | lpChar | Conversion from char * to wchar_t *. Use of invalid string can lead to undefined behavior. | -| WcharCharConversion.cpp:28:21:28:27 | lpWchar | Conversion from wchar_t * to LPCSTR. Use of invalid string can lead to undefined behavior. | -| WcharCharConversion.cpp:29:15:29:21 | lpWchar | Conversion from wchar_t * to LPSTR. Use of invalid string can lead to undefined behavior. | -| WcharCharConversion.cpp:30:23:30:28 | lpChar | Conversion from char * to LPCWSTR. Use of invalid string can lead to undefined behavior. | -| WcharCharConversion.cpp:31:17:31:22 | lpChar | Conversion from char * to LPWSTR. Use of invalid string can lead to undefined behavior. | +| WcharCharConversion.cpp:21:20:21:22 | array to pointer conversion | Conversion from const char * to LPWSTR. Use of invalid string can lead to undefined behavior. | +| WcharCharConversion.cpp:22:20:22:25 | lpcstr | Conversion from LPCSTR to LPWSTR. Use of invalid string can lead to undefined behavior. | +| WcharCharConversion.cpp:24:22:24:27 | lpChar | Conversion from char * to wchar_t *. Use of invalid string can lead to undefined behavior. | +| WcharCharConversion.cpp:26:23:26:28 | lpChar | Conversion from char * to LPCWSTR. Use of invalid string can lead to undefined behavior. | +| WcharCharConversion.cpp:27:17:27:22 | lpChar | Conversion from char * to LPWSTR. Use of invalid string can lead to undefined behavior. |