Skip to content

Commit 6e49891

Browse files
committed
C++: Accept Microsoft/non-Microsoft format specifiers on the opposite platform.
1 parent a1c38b7 commit 6e49891

File tree

10 files changed

+64
-20
lines changed

10 files changed

+64
-20
lines changed

cpp/ql/src/Likely Bugs/Format/WrongTypeFormatArguments.ql

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,24 @@ private predicate formattingFunctionCallExpectedType(
3232
)
3333
}
3434

35+
/**
36+
* Holds if the argument corresponding to the `pos` conversion specifier
37+
* of `ffc` could alternatively have type `expected`, for example on a different
38+
* platform.
39+
*/
40+
pragma[noopt]
41+
private predicate formattingFunctionCallAlternateType(
42+
FormattingFunctionCall ffc, int pos, Type expected
43+
) {
44+
exists(FormattingFunction f, int i, FormatLiteral fl |
45+
ffc instanceof FormattingFunctionCall and
46+
ffc.getTarget() = f and
47+
f.getFormatParameterIndex() = i and
48+
ffc.getArgument(i) = fl and
49+
fl.getConversionTypeAlternate(pos) = expected
50+
)
51+
}
52+
3553
/**
3654
* Holds if the argument corresponding to the `pos` conversion specifier
3755
* of `ffc` is expected to have type `expected` and the corresponding
@@ -73,6 +91,7 @@ class ExpectedType extends Type {
7391
exists(Type t |
7492
(
7593
formatArgType(_, _, t, _, _) or
94+
formattingFunctionCallAlternateType(_, _, t) or
7695
formatOtherArgType(_, _, t, _, _)
7796
) and
7897
this = t.getUnspecifiedType()
@@ -91,7 +110,11 @@ class ExpectedType extends Type {
91110
*/
92111
predicate trivialConversion(ExpectedType expected, Type actual) {
93112
exists(Type exp, Type act |
94-
formatArgType(_, _, exp, _, act) and
113+
(
114+
formatArgType(_, _, exp, _, _) or
115+
formattingFunctionCallAlternateType(_, _, exp)
116+
) and
117+
formatArgType(_, _, _, _, act) and
95118
expected = exp.getUnspecifiedType() and
96119
actual = act.getUnspecifiedType()
97120
) and
@@ -148,7 +171,10 @@ where
148171
(
149172
formatArgType(ffc, n, expected, arg, actual) and
150173
not exists(Type anyExpected |
151-
formatArgType(ffc, n, anyExpected, arg, actual) and
174+
(
175+
formatArgType(ffc, n, anyExpected, arg, actual) or
176+
formattingFunctionCallAlternateType(ffc, n, anyExpected)
177+
) and
152178
trivialConversion(anyExpected.getUnspecifiedType(), actual.getUnspecifiedType())
153179
)
154180
or

cpp/ql/src/semmle/code/cpp/commons/Printf.qll

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -869,6 +869,33 @@ class FormatLiteral extends Literal {
869869
)
870870
}
871871

872+
/**
873+
* Gets an alternate argument type that would be required by the nth
874+
* conversion specifier on a Microsoft or non-Microsoft platform, opposite
875+
* to that of the snapshot. This may be useful for answering 'what might
876+
* happen' questions.
877+
*/
878+
Type getConversionTypeAlternate(int n) {
879+
exists(string len, string conv |
880+
this.parseConvSpec(n, _, _, _, _, _, len, conv) and
881+
(len != "l" and len != "w" and len != "h") and
882+
getUse().getTarget().(FormattingFunction).getFormatCharType().getSize() > 1 and // wide function
883+
(
884+
conv = "c" and
885+
result = getNonDefaultCharType()
886+
or
887+
conv = "C" and
888+
result = getDefaultCharType()
889+
or
890+
conv = "s" and
891+
result.(PointerType).getBaseType() = getNonDefaultCharType()
892+
or
893+
conv = "S" and
894+
result.(PointerType).getBaseType() = getDefaultCharType()
895+
)
896+
)
897+
}
898+
872899
/**
873900
* Holds if the nth conversion specifier of this format string (if `mode = 2`), it's
874901
* minimum field width (if `mode = 0`) or it's precision (if `mode = 1`) requires a

cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/WrongTypeFormatArguments.expected

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,8 @@
33
| tests.cpp:21:15:21:21 | Hello | This argument should be of type 'char16_t *' but is of type 'char *' |
44
| tests.cpp:21:15:21:21 | Hello | This argument should be of type 'wchar_t *' but is of type 'char *' |
55
| tests.cpp:26:17:26:24 | Hello | This argument should be of type 'char *' but is of type 'char16_t *' |
6-
| tests.cpp:27:17:27:24 | Hello | This argument should be of type 'char *' but is of type 'wchar_t *' |
7-
| tests.cpp:29:17:29:23 | Hello | This argument should be of type 'wchar_t *' but is of type 'char *' |
86
| tests.cpp:30:17:30:24 | Hello | This argument should be of type 'wchar_t *' but is of type 'char16_t *' |
9-
| tests.cpp:34:36:34:43 | Hello | This argument should be of type 'char *' but is of type 'char16_t *' |
107
| tests.cpp:35:36:35:43 | Hello | This argument should be of type 'char *' but is of type 'wchar_t *' |
11-
| tests.cpp:37:36:37:42 | Hello | This argument should be of type 'char16_t *' but is of type 'char *' |
128
| tests.cpp:39:36:39:43 | Hello | This argument should be of type 'char16_t *' but is of type 'wchar_t *' |
139
| tests.cpp:42:37:42:44 | Hello | This argument should be of type 'char *' but is of type 'char16_t *' |
1410
| tests.cpp:43:37:43:44 | Hello | This argument should be of type 'char *' but is of type 'wchar_t *' |

cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/tests.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,17 +24,17 @@ void tests() {
2424

2525
wprintf(L"%s", "Hello"); // GOOD
2626
wprintf(L"%s", u"Hello"); // BAD: expecting char
27-
wprintf(L"%s", L"Hello"); // BAD: expecting char
27+
wprintf(L"%s", L"Hello"); // BAD: expecting char [NOT DETECTED; correct on Microsoft platforms]
2828

29-
wprintf(L"%S", "Hello"); // BAD: expecting wchar_t
29+
wprintf(L"%S", "Hello"); // BAD: expecting wchar_t [NOT DETECTED; correct on Microsoft platforms]
3030
wprintf(L"%S", u"Hello"); // BAD: expecting wchar_t
3131
wprintf(L"%S", L"Hello"); // GOOD
3232

3333
swprintf(buffer, BUF_SIZE, u"%s", "Hello"); // GOOD
34-
swprintf(buffer, BUF_SIZE, u"%s", u"Hello"); // BAD: expecting char
34+
swprintf(buffer, BUF_SIZE, u"%s", u"Hello"); // BAD: expecting char [NOT DETECTED; correct on Microsoft platforms]
3535
swprintf(buffer, BUF_SIZE, u"%s", L"Hello"); // BAD: expecting char
3636

37-
swprintf(buffer, BUF_SIZE, u"%S", "Hello"); // BAD: expecting char16_t
37+
swprintf(buffer, BUF_SIZE, u"%S", "Hello"); // BAD: expecting char16_t [NOT DETECTED; correct on Microsoft platforms]
3838
swprintf(buffer, BUF_SIZE, u"%S", u"Hello"); // GOOD
3939
swprintf(buffer, BUF_SIZE, u"%S", L"Hello"); // BAD: expecting char16_t
4040

Original file line numberDiff line numberDiff line change
@@ -1,3 +1,2 @@
1-
| printf.cpp:31:31:31:37 | test | This argument should be of type 'char *' but is of type 'char16_t *' |
21
| printf.cpp:43:29:43:35 | test | This argument should be of type 'char *' but is of type 'char16_t *' |
32
| printf.cpp:50:29:50:35 | test | This argument should be of type 'char16_t *' but is of type 'wchar_t *' |

cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_two_byte_wprintf/printf.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ int sprintf(char *dest, char *format, ...);
2828
void test1() {
2929
WCHAR string[20];
3030

31-
swprintf(string, u"test %s", u"test"); // BAD: `char16_t` string parameter read as `char` string
31+
swprintf(string, u"test %s", u"test"); // BAD: `char16_t` string parameter read as `char` string [NOT DETECTED; correct on Microsoft platforms]
3232
}
3333

3434
void test2() {

cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_unsigned_chars/WrongTypeFormatArguments.expected

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,6 @@
1111
| printf1.h:45:18:45:20 | ull | This argument should be of type 'unsigned int' but is of type 'unsigned long long' |
1212
| printf1.h:46:18:46:20 | ull | This argument should be of type 'unsigned int' but is of type 'unsigned long long' |
1313
| printf1.h:130:18:130:18 | 0 | This argument should be of type 'void *' but is of type 'int' |
14-
| printf1.h:154:18:154:19 | wc | This argument should be of type 'char *' but is of type 'wchar_t *' |
15-
| printf1.h:155:18:155:18 | c | This argument should be of type 'wchar_t *' but is of type 'char *' |
1614
| printf1.h:168:19:168:19 | i | This argument should be of type 'long long' but is of type 'int' |
1715
| printf1.h:169:19:169:20 | ui | This argument should be of type 'unsigned long long' but is of type 'unsigned int' |
1816
| real_world.h:61:21:61:22 | & ... | This argument should be of type 'int *' but is of type 'short *' |

cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_unsigned_chars/printf1.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -151,8 +151,8 @@ void test_chars(char c, wchar_t wc, wint_t wt)
151151
void test_ws(char *c, wchar_t *wc)
152152
{
153153
wprintf(L"%s", c); // GOOD
154-
wprintf(L"%s", wc); // BAD
155-
wprintf(L"%S", c); // BAD
154+
wprintf(L"%s", wc); // BAD [NOT DETECTED; correct on Microsoft platforms]
155+
wprintf(L"%S", c); // BAD [NOT DETECTED; correct on Microsoft platforms]
156156
wprintf(L"%S", wc); // GOOD
157157
}
158158

cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Microsoft/WrongTypeFormatArguments.expected

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,6 @@
1919
| printf1.h:116:16:116:24 | myString3 | This argument should be of type '__wchar_t *' but is of type 'int *' |
2020
| printf1.h:117:16:117:24 | myString4 | This argument should be of type '__wchar_t *' but is of type 'int *' |
2121
| printf1.h:130:18:130:18 | 0 | This argument should be of type 'void *' but is of type 'int' |
22-
| printf1.h:153:18:153:18 | c | This argument should be of type '__wchar_t *' but is of type 'char *' |
23-
| printf1.h:156:18:156:19 | wc | This argument should be of type 'char *' but is of type '__wchar_t *' |
2422
| printf1.h:181:21:181:22 | ll | This argument should be of type 'int' but is of type 'long long' |
2523
| printf1.h:182:21:182:23 | ull | This argument should be of type 'unsigned int' but is of type 'unsigned long long' |
2624
| printf1.h:185:21:185:23 | i64 | This argument should be of type 'int' but is of type 'long long' |

cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Microsoft/printf1.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -150,10 +150,10 @@ void test_chars(char c, wchar_t wc, wint_t wt)
150150

151151
void test_ws(char *c, wchar_t *wc, wint_t *wt)
152152
{
153-
wprintf(L"%s", c); // BAD
153+
wprintf(L"%s", c); // BAD [NOT DETECTED; correct on non-Microsoft platforms]
154154
wprintf(L"%s", wc); // GOOD
155155
wprintf(L"%S", c); // GOOD
156-
wprintf(L"%S", wc); // BAD
156+
wprintf(L"%S", wc); // BAD [NOT DETECTED; correct on non-Microsoft platforms]
157157
}
158158

159159
void fun4()

0 commit comments

Comments
 (0)