Skip to content

Commit 649c883

Browse files
authored
Merge pull request #20014 from jketema/wchar
C++: Do not alert on unreachable code in `cpp/incorrect-string-type-conversion`
2 parents 0a18db8 + 990b7f0 commit 649c883

File tree

5 files changed

+55
-15
lines changed

5 files changed

+55
-15
lines changed

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
import cpp
1616
import semmle.code.cpp.controlflow.Guards
17+
import semmle.code.cpp.ir.IR
1718

1819
class WideCharPointerType extends PointerType {
1920
WideCharPointerType() { this.getBaseType() instanceof WideCharType }
@@ -108,7 +109,9 @@ where
108109
// Avoid cases where the cast is guarded by a check to determine if
109110
// unicode encoding is enabled in such a way to disallow the dangerous cast
110111
// at runtime.
111-
not isLikelyDynamicallyChecked(e1)
112+
not isLikelyDynamicallyChecked(e1) and
113+
// Avoid cases in unreachable blocks.
114+
any(EnterFunctionInstruction e).getASuccessor+().getAst() = e1
112115
select e1,
113116
"Conversion from " + e1.getType().toString() + " to " + e2.getType().toString() +
114117
". Use of invalid string can lead to undefined behavior."
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* The `cpp/incorrect-string-type-conversion` query no longer alerts on incorrect type conversions that occur in unreachable code.

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

Lines changed: 44 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,13 @@ void Test()
1818
wchar_t *lpWchar = NULL;
1919
LPCSTR lpcstr = "b";
2020

21-
lpWchar = (LPWSTR)"a"; // BUG
22-
lpWchar = (LPWSTR)lpcstr; // BUG
21+
lpWchar = (LPWSTR)"a"; // $ Alert
22+
lpWchar = (LPWSTR)lpcstr; // $ Alert
2323

24-
lpWchar = (wchar_t*)lpChar; // BUG
24+
lpWchar = (wchar_t*)lpChar; // $ Alert
2525

26-
fconstWChar((LPCWSTR)lpChar); // BUG
27-
fWChar((LPWSTR)lpChar); // BUG
26+
fconstWChar((LPCWSTR)lpChar); // $ Alert
27+
fWChar((LPWSTR)lpChar); // $ Alert
2828

2929
lpChar = (LPSTR)"a"; // Valid
3030
lpWchar = (LPWSTR)L"a"; // Valid
@@ -79,33 +79,64 @@ void CheckedConversionFalsePositiveTest3(unsigned short flags, LPTSTR buffer)
7979
if(flags & UNICODE)
8080
lpWchar = (LPWSTR)buffer; // GOOD
8181
else
82-
lpWchar = (LPWSTR)buffer; // BUG
82+
lpWchar = (LPWSTR)buffer; // $ Alert
8383

8484
if((flags & UNICODE) == 0x8)
8585
lpWchar = (LPWSTR)buffer; // GOOD
8686
else
87-
lpWchar = (LPWSTR)buffer; // BUG
87+
lpWchar = (LPWSTR)buffer; // $ Alert
8888

8989
if((flags & UNICODE) != 0x8)
90-
lpWchar = (LPWSTR)buffer; // BUG
90+
lpWchar = (LPWSTR)buffer; // $ Alert
9191
else
9292
lpWchar = (LPWSTR)buffer; // GOOD
9393

9494
// Bad operator precedence
9595
if(flags & UNICODE == 0x8)
96-
lpWchar = (LPWSTR)buffer; // BUG
96+
lpWchar = (LPWSTR)buffer; // $ Alert
9797
else
98-
lpWchar = (LPWSTR)buffer; // BUG
98+
lpWchar = (LPWSTR)buffer; // $ Alert
9999

100100
if((flags & UNICODE) != 0)
101101
lpWchar = (LPWSTR)buffer; // GOOD
102102
else
103-
lpWchar = (LPWSTR)buffer; // BUG
103+
lpWchar = (LPWSTR)buffer; // $ Alert
104104

105105
if((flags & UNICODE) == 0)
106-
lpWchar = (LPWSTR)buffer; // BUG
106+
lpWchar = (LPWSTR)buffer; // $ Alert
107107
else
108108
lpWchar = (LPWSTR)buffer; // GOOD
109109

110-
lpWchar = (LPWSTR)buffer; // BUG
110+
lpWchar = (LPWSTR)buffer; // $ Alert
111+
}
112+
113+
typedef unsigned long long size_t;
114+
115+
size_t wcslen(const wchar_t *str);
116+
size_t strlen(const char* str);
117+
118+
template<typename C>
119+
size_t str_len(const C *str) {
120+
if (sizeof(C) != 1) {
121+
return wcslen((const wchar_t *)str); // GOOD -- unreachable code
122+
}
123+
124+
return strlen((const char *)str);
125+
}
126+
127+
template<typename C>
128+
size_t wrong_str_len(const C *str) {
129+
if (sizeof(C) == 1) {
130+
return wcslen((const wchar_t *)str); // $ Alert
131+
}
132+
133+
return strlen((const char *)str);
134+
}
135+
136+
void test_str_len(const wchar_t *wstr, const char *str) {
137+
size_t len =
138+
str_len(wstr) +
139+
str_len(str) +
140+
wrong_str_len(wstr) +
141+
wrong_str_len(str);
111142
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,3 +11,4 @@
1111
| WcharCharConversion.cpp:103:21:103:26 | buffer | Conversion from LPTSTR to LPWSTR. Use of invalid string can lead to undefined behavior. |
1212
| WcharCharConversion.cpp:106:21:106:26 | buffer | Conversion from LPTSTR to LPWSTR. Use of invalid string can lead to undefined behavior. |
1313
| WcharCharConversion.cpp:110:20:110:25 | buffer | Conversion from LPTSTR to LPWSTR. Use of invalid string can lead to undefined behavior. |
14+
| WcharCharConversion.cpp:130:34:130:36 | str | Conversion from const char * to const wchar_t *. Use of invalid string can lead to undefined behavior. |
Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1,2 @@
1-
Security/CWE/CWE-704/WcharCharConversion.ql
1+
query: Security/CWE/CWE-704/WcharCharConversion.ql
2+
postprocess: utils/test/InlineExpectationsTestQuery.ql

0 commit comments

Comments
 (0)