Skip to content

Commit 230724c

Browse files
raulgarciamsftraulgarciamsft
committed
Updates based on feedback
1 parent 253b8d1 commit 230724c

File tree

6 files changed

+38
-45
lines changed

6 files changed

+38
-45
lines changed

‎.gitignore‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,3 +14,4 @@
1414
/.vs/ProjectSettings.json
1515

1616
/.vs/ql/v15/.suo
17+
/.vs/VSWorkspaceState.json

‎cpp/ql/src/Security/CWE/CWE-704/WcharCharConversion.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
1-
LPWSTR pSrc;
1+
wchar_t* pSrc;
22

3-
pSrc = (LPWSTR)"a";
3+
pSrc = (wchar_t*)"a"; // casting a byte-string literal "a" to a wide-character string

‎cpp/ql/src/Security/CWE/CWE-704/WcharCharConversion.qhelp‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,22 +4,32 @@
44
<qhelp>
55

66
<overview>
7-
<p>This rule indicates a potentially incorrect cast from/to an ANSI string (<code>char *</code>) to/from a Unicode string (<code>wchar_t *</code>).</p>
7+
<p>This rule indicates a potentially incorrect cast from an byte string (<code>char *</code>) to a wide-character string (<code>wchar_t *</code>).</p>
88
<p>This cast might yield strings that are not correctly terminated; including potential buffer overruns when using such strings with some dangerous APIs.</p>
99
</overview>
1010

1111
<recommendation>
12-
<p>Do not explicitly casting ANSI strings to/from Unicode strings.</p>
12+
<p>Do not explicitly cast byte strings to wide-character strings.</p>
13+
<p>For string literals, prepend the literal string with the letter "L" to indicate that the string is a wide-character string (<code>wchar_t *</code>).</p>
14+
<p>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.</p>
1315
</recommendation>
1416

1517
<example>
16-
<p>In the following example, an ANSI string literal (<code>"a"</code>) is casted as a Unicode string.</p>
18+
<p>In the following example, an byte string literal (<code>"a"</code>) is cast to a wide-character string.</p>
1719
<sample src="WcharCharConversion.cpp" />
1820

19-
<p>To fix this issue, prepend the literal with the letter "L" (<code>L"a"</code>) to define it as a Unicode string.</p>
21+
<p>To fix this issue, prepend the literal with the letter "L" (<code>L"a"</code>) to define it as a wide-character string.</p>
2022
</example>
2123

2224
<references>
25+
<li>
26+
General resources:
27+
<a href="https://en.cppreference.com/w/cpp/string/multibyte/mbstowcs">std::mbstowcs</a>
28+
</li>
29+
<li>
30+
Microsoft specific resources:
31+
<a href="https://docs.microsoft.com/en-us/windows/desktop/Intl/security-considerations--international-features">Security Considerations: International Features</a>
32+
</li>
2333
</references>
2434

2535
</qhelp>

‎cpp/ql/src/Security/CWE/CWE-704/WcharCharConversion.ql‎

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -21,18 +21,9 @@ class WideCharPointerType extends PointerType {
2121

2222
from Expr e1, Cast e2
2323
where
24-
e2 = e1.getConversion()
25-
and
26-
(
27-
exists( WideCharPointerType w, CharPointerType c |
28-
w = e1.getType().getUnspecifiedType().(PointerType)
29-
and c = e2.getType().getUnspecifiedType().(PointerType)
30-
)
31-
or exists
32-
(
33-
WideCharPointerType w, CharPointerType c |
34-
w = e2.getType().getUnspecifiedType().(PointerType)
35-
and c = e1.getType().getUnspecifiedType().(PointerType)
36-
)
24+
e2 = e1.getConversion() and
25+
exists(WideCharPointerType w, CharPointerType c |
26+
w = e2.getType().getUnspecifiedType().(PointerType) and
27+
c = e1.getType().getUnspecifiedType().(PointerType)
3728
)
3829
select e1, "Conversion from " + e1.getType().toString() + " to " + e2.getType().toString() + ". Use of invalid string can lead to undefined behavior."

‎cpp/ql/test/query-tests/Security/CWE/CWE-704/WcharCharConversion.cpp‎

Lines changed: 12 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -9,32 +9,26 @@ typedef CONST WCHAR *LPCWSTR;
99
typedef CHAR *LPSTR;
1010
typedef CONST CHAR *LPCSTR;
1111

12-
void fconstChar(LPCSTR p) {}
13-
void fChar(LPSTR p) {}
1412
void fconstWChar(LPCWSTR p) {}
1513
void fWChar(LPWSTR p) {}
1614

1715
void Test()
1816
{
19-
char *lpChar = NULL;
20-
wchar_t *lpWchar = NULL;
17+
char *lpChar = NULL;
18+
wchar_t *lpWchar = NULL;
19+
LPCSTR lpcstr = "b";
2120

22-
lpChar = (LPSTR)L"a"; // BUG
23-
lpWchar = (LPWSTR)"a"; // BUG
21+
lpWchar = (LPWSTR)"a"; // BUG
22+
lpWchar = (LPWSTR)lpcstr; // BUG
2423

25-
lpChar = (char*)lpWchar; // BUG
26-
lpWchar = (wchar_t*)lpChar; // BUG
24+
lpWchar = (wchar_t*)lpChar; // BUG
2725

28-
fconstChar((LPCSTR)lpWchar); // BUG
29-
fChar((LPSTR)lpWchar); // BUG
30-
fconstWChar((LPCWSTR)lpChar); // BUG
31-
fWChar((LPWSTR)lpChar); // BUG
26+
fconstWChar((LPCWSTR)lpChar); // BUG
27+
fWChar((LPWSTR)lpChar); // BUG
3228

33-
lpChar = (LPSTR)"a"; // Valid
34-
lpWchar = (LPWSTR)L"a"; // Valid
29+
lpChar = (LPSTR)"a"; // Valid
30+
lpWchar = (LPWSTR)L"a"; // Valid
3531

36-
fconstChar((LPCSTR)lpChar); // Valid
37-
fChar(lpChar); // Valid
38-
fconstWChar((LPCWSTR)lpWchar); // Valid
39-
fWChar(lpWchar); // Valid
32+
fconstWChar((LPCWSTR)lpWchar); // Valid
33+
fWChar(lpWchar); // Valid
4034
}

‎cpp/ql/test/query-tests/Security/CWE/CWE-704/WcharCharConversion.expected‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,5 @@
1-
| 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. |
2-
| 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. |
3-
| WcharCharConversion.cpp:25:18:25:24 | lpWchar | Conversion from wchar_t * to char *. Use of invalid string can lead to undefined behavior. |
4-
| WcharCharConversion.cpp:26:22:26:27 | lpChar | Conversion from char * to wchar_t *. Use of invalid string can lead to undefined behavior. |
5-
| WcharCharConversion.cpp:28:21:28:27 | lpWchar | Conversion from wchar_t * to LPCSTR. Use of invalid string can lead to undefined behavior. |
6-
| WcharCharConversion.cpp:29:15:29:21 | lpWchar | Conversion from wchar_t * to LPSTR. Use of invalid string can lead to undefined behavior. |
7-
| WcharCharConversion.cpp:30:23:30:28 | lpChar | Conversion from char * to LPCWSTR. Use of invalid string can lead to undefined behavior. |
8-
| WcharCharConversion.cpp:31:17:31:22 | lpChar | Conversion from char * to LPWSTR. Use of invalid string can lead to undefined behavior. |
1+
| 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. |
2+
| WcharCharConversion.cpp:22:20:22:25 | lpcstr | Conversion from LPCSTR to LPWSTR. Use of invalid string can lead to undefined behavior. |
3+
| WcharCharConversion.cpp:24:22:24:27 | lpChar | Conversion from char * to wchar_t *. Use of invalid string can lead to undefined behavior. |
4+
| WcharCharConversion.cpp:26:23:26:28 | lpChar | Conversion from char * to LPCWSTR. Use of invalid string can lead to undefined behavior. |
5+
| WcharCharConversion.cpp:27:17:27:22 | lpChar | Conversion from char * to LPWSTR. Use of invalid string can lead to undefined behavior. |

0 commit comments

Comments
 (0)