From b3fd7ab757e7f56ca01f6d80e9f84a98cd7ed0ba Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Tue, 2 Apr 2019 16:58:23 +0100 Subject: [PATCH 1/3] CPP: Add test cases. --- .../WrongTypeFormatArguments.expected | 6 ++++ .../Linux_signed_chars/printf1.h | 33 +++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_signed_chars/WrongTypeFormatArguments.expected b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_signed_chars/WrongTypeFormatArguments.expected index 568bb88ebeb3..07f73eac1c8a 100644 --- a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_signed_chars/WrongTypeFormatArguments.expected +++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_signed_chars/WrongTypeFormatArguments.expected @@ -11,6 +11,12 @@ | printf1.h:45:18:45:20 | ull | This argument should be of type 'unsigned int' but is of type 'unsigned long long' | | printf1.h:46:18:46:20 | ull | This argument should be of type 'unsigned int' but is of type 'unsigned long long' | | printf1.h:47:19:47:21 | ull | This argument should be of type 'unsigned int' but is of type 'unsigned long long' | +| printf1.h:112:18:112:19 | ld | This argument should be of type 'double' but is of type 'long double' | +| printf1.h:113:17:113:17 | d | This argument should be of type 'long double' but is of type 'double' | +| printf1.h:122:17:122:19 | lli | This argument should be of type 'int' but is of type 'long long' | +| printf1.h:123:17:123:18 | li | This argument should be of type 'int' but is of type 'long' | +| printf1.h:132:17:132:20 | ulli | This argument should be of type 'unsigned int' but is of type 'unsigned long long' | +| printf1.h:133:17:133:19 | uli | This argument should be of type 'unsigned int' but is of type 'unsigned long' | | real_world.h:61:21:61:22 | & ... | This argument should be of type 'int *' but is of type 'short *' | | real_world.h:62:22:62:23 | & ... | This argument should be of type 'short *' but is of type 'int *' | | real_world.h:63:22:63:24 | & ... | This argument should be of type 'short *' but is of type 'unsigned int *' | diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_signed_chars/printf1.h b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_signed_chars/printf1.h index 824cd2d8aeda..997e14a27dc2 100644 --- a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_signed_chars/printf1.h +++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_signed_chars/printf1.h @@ -101,3 +101,36 @@ void fun1(unsigned char* a, unsigned char* b) { printf("%td\n", pdt); // GOOD printf("%td\n", a-b); // GOOD } + +void extensions() +{ + { + long double ld; + double d; + + printf("%Lg", ld); // GOOD + printf("%llg", ld); // GOOD (nonstandard equivalent to %Lg) [FALSE POSITIVE] + printf("%Lg", d); // BAD (should be %g) + printf("%llg", d); // BAD (should be %g) [NOT DETECTED] + } + + { + long long int lli; + long int li; + + printf("%lld", lli); // GOOD + printf("%Ld", lli); // GOOD (nonstandard equivalent to %lld) [FALSE POSITIVE] + printf("%Ld", li); // BAD (should be %ld) + printf("%lld", li); // BAD (should be %ld) [NOT DETECTED] + } + + { + unsigned long long int ulli; + unsigned long int uli; + + printf("%llu", ulli); // GOOD + printf("%Lu", ulli); // GOOD (nonstandard equivalent to %llu) [FALSE POSITIVE] + printf("%Lu", uli); // BAD (should be %lu) + printf("%llu", uli); // BAD (should be %lu) [NOT DETECTED] + } +} From d4c931cf116771ca45dc7f30872847d5c597f139 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Tue, 2 Apr 2019 17:32:20 +0100 Subject: [PATCH 2/3] CPP: Permit %Ld and similar. --- cpp/ql/src/semmle/code/cpp/commons/Printf.qll | 10 ++++------ .../WrongTypeFormatArguments.expected | 7 +------ .../Linux_signed_chars/printf1.h | 14 +++++++------- .../WrongTypeFormatArguments.expected | 1 - .../Linux_unsigned_chars/printf1.h | 2 +- .../Microsoft/WrongTypeFormatArguments.expected | 1 - .../WrongTypeFormatArguments/Microsoft/printf1.h | 2 +- .../WrongTypeFormatArguments.expected | 1 - .../Microsoft_no_wchar/printf1.h | 2 +- 9 files changed, 15 insertions(+), 25 deletions(-) diff --git a/cpp/ql/src/semmle/code/cpp/commons/Printf.qll b/cpp/ql/src/semmle/code/cpp/commons/Printf.qll index 7d96ffbd3a90..5e8f81a46114 100644 --- a/cpp/ql/src/semmle/code/cpp/commons/Printf.qll +++ b/cpp/ql/src/semmle/code/cpp/commons/Printf.qll @@ -572,9 +572,8 @@ class FormatLiteral extends Literal { ((len="hh" and result instanceof IntType) or (len="h" and result instanceof IntType) or (len="l" and result = this.getLongType()) - or ((len="ll" or len="q") + or ((len="ll" or len="L" or len="q") and result instanceof LongLongType) - or (len="L" and result instanceof IntType) // doesn't affect integral conversion or (len="j" and result = this.getIntmax_t()) or ((len="z" or len="Z") and (result = this.getSize_t() or result = this.getSsize_t())) @@ -599,9 +598,8 @@ class FormatLiteral extends Literal { ((len="hh" and result instanceof CharType) or (len="h" and result instanceof ShortType) or (len="l" and result = this.getLongType()) - or ((len="ll" or len="q") + or ((len="ll" or len="L" or len="q") and result instanceof LongLongType) - or (len="L" and result instanceof IntType) // doesn't affect integral conversion or (len="j" and result = this.getIntmax_t()) or ((len="z" or len="Z") and (result = this.getSize_t() or result = this.getSsize_t())) @@ -622,7 +620,7 @@ class FormatLiteral extends Literal { */ FloatingPointType getFloatingPointConversion(int n) { exists(string len | len = this.getLength(n) and - if len="L" then + if (len="L" or len="ll") then result instanceof LongDoubleType else result instanceof DoubleType) @@ -638,7 +636,7 @@ class FormatLiteral extends Literal { (len="hh" and base instanceof CharType) or (len="h" and base instanceof ShortType) or (len="l" and base = this.getLongType()) - or (len="ll" and base instanceof LongLongType) + or ((len="ll" or len="L") and base instanceof LongLongType) or (len="q" and base instanceof LongLongType) ) and base.isSigned() and base = result.getBaseType() diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_signed_chars/WrongTypeFormatArguments.expected b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_signed_chars/WrongTypeFormatArguments.expected index 07f73eac1c8a..648fda9c0cc2 100644 --- a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_signed_chars/WrongTypeFormatArguments.expected +++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_signed_chars/WrongTypeFormatArguments.expected @@ -10,13 +10,8 @@ | printf1.h:44:18:44:20 | ull | This argument should be of type 'int' but is of type 'unsigned long long' | | printf1.h:45:18:45:20 | ull | This argument should be of type 'unsigned int' but is of type 'unsigned long long' | | printf1.h:46:18:46:20 | ull | This argument should be of type 'unsigned int' but is of type 'unsigned long long' | -| printf1.h:47:19:47:21 | ull | This argument should be of type 'unsigned int' but is of type 'unsigned long long' | -| printf1.h:112:18:112:19 | ld | This argument should be of type 'double' but is of type 'long double' | | printf1.h:113:17:113:17 | d | This argument should be of type 'long double' but is of type 'double' | -| printf1.h:122:17:122:19 | lli | This argument should be of type 'int' but is of type 'long long' | -| printf1.h:123:17:123:18 | li | This argument should be of type 'int' but is of type 'long' | -| printf1.h:132:17:132:20 | ulli | This argument should be of type 'unsigned int' but is of type 'unsigned long long' | -| printf1.h:133:17:133:19 | uli | This argument should be of type 'unsigned int' but is of type 'unsigned long' | +| printf1.h:114:18:114:18 | d | This argument should be of type 'long double' but is of type 'double' | | real_world.h:61:21:61:22 | & ... | This argument should be of type 'int *' but is of type 'short *' | | real_world.h:62:22:62:23 | & ... | This argument should be of type 'short *' but is of type 'int *' | | real_world.h:63:22:63:24 | & ... | This argument should be of type 'short *' but is of type 'unsigned int *' | diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_signed_chars/printf1.h b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_signed_chars/printf1.h index 997e14a27dc2..387811f0d557 100644 --- a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_signed_chars/printf1.h +++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_signed_chars/printf1.h @@ -44,7 +44,7 @@ void f(char *s, int i, unsigned char *us, const char *cs, signed char *ss, char printf("%d", ull); // not ok (unsigned long long -> int) printf("%u", ull); // not ok (unsigned long long -> unsigned int) printf("%x", ull); // not ok (unsigned long long -> unsigned int) - printf("%Lx", ull); // not ok (unsigned long long -> unsigned int) + printf("%Lx", ull); // ok printf("%llx", ull); // ok } @@ -109,9 +109,9 @@ void extensions() double d; printf("%Lg", ld); // GOOD - printf("%llg", ld); // GOOD (nonstandard equivalent to %Lg) [FALSE POSITIVE] + printf("%llg", ld); // GOOD (nonstandard equivalent to %Lg) printf("%Lg", d); // BAD (should be %g) - printf("%llg", d); // BAD (should be %g) [NOT DETECTED] + printf("%llg", d); // BAD (should be %g) } { @@ -119,8 +119,8 @@ void extensions() long int li; printf("%lld", lli); // GOOD - printf("%Ld", lli); // GOOD (nonstandard equivalent to %lld) [FALSE POSITIVE] - printf("%Ld", li); // BAD (should be %ld) + printf("%Ld", lli); // GOOD (nonstandard equivalent to %lld) + printf("%Ld", li); // BAD (should be %ld) [NOT DETECTED] printf("%lld", li); // BAD (should be %ld) [NOT DETECTED] } @@ -129,8 +129,8 @@ void extensions() unsigned long int uli; printf("%llu", ulli); // GOOD - printf("%Lu", ulli); // GOOD (nonstandard equivalent to %llu) [FALSE POSITIVE] - printf("%Lu", uli); // BAD (should be %lu) + printf("%Lu", ulli); // GOOD (nonstandard equivalent to %llu) + printf("%Lu", uli); // BAD (should be %lu) [NOT DETECTED] printf("%llu", uli); // BAD (should be %lu) [NOT DETECTED] } } diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_unsigned_chars/WrongTypeFormatArguments.expected b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_unsigned_chars/WrongTypeFormatArguments.expected index 1aca7fbf9ced..a55c16faf904 100644 --- a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_unsigned_chars/WrongTypeFormatArguments.expected +++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_unsigned_chars/WrongTypeFormatArguments.expected @@ -10,7 +10,6 @@ | printf1.h:44:18:44:20 | ull | This argument should be of type 'int' but is of type 'unsigned long long' | | printf1.h:45:18:45:20 | ull | This argument should be of type 'unsigned int' but is of type 'unsigned long long' | | printf1.h:46:18:46:20 | ull | This argument should be of type 'unsigned int' but is of type 'unsigned long long' | -| printf1.h:47:19:47:21 | ull | This argument should be of type 'unsigned int' but is of type 'unsigned long long' | | printf1.h:126:18:126:19 | wc | This argument should be of type 'char *' but is of type 'wchar_t *' | | printf1.h:127:18:127:18 | c | This argument should be of type 'wchar_t *' but is of type 'char *' | | real_world.h:61:21:61:22 | & ... | This argument should be of type 'int *' but is of type 'short *' | diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_unsigned_chars/printf1.h b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_unsigned_chars/printf1.h index 1857ad23e157..0af5d8163f98 100644 --- a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_unsigned_chars/printf1.h +++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_unsigned_chars/printf1.h @@ -44,7 +44,7 @@ void f(char *s, int i, unsigned char *us, const char *cs, signed char *ss, char printf("%d", ull); // not ok (unsigned long long -> int) printf("%u", ull); // not ok (unsigned long long -> unsigned int) printf("%x", ull); // not ok (unsigned long long -> unsigned int) - printf("%Lx", ull); // not ok (unsigned long long -> unsigned int) + printf("%Lx", ull); // ok printf("%llx", ull); // ok } diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Microsoft/WrongTypeFormatArguments.expected b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Microsoft/WrongTypeFormatArguments.expected index cc41cd7e7428..5afaa3bddbe0 100644 --- a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Microsoft/WrongTypeFormatArguments.expected +++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Microsoft/WrongTypeFormatArguments.expected @@ -10,7 +10,6 @@ | printf1.h:44:18:44:20 | ull | This argument should be of type 'int' but is of type 'unsigned long long' | | printf1.h:45:18:45:20 | ull | This argument should be of type 'unsigned int' but is of type 'unsigned long long' | | printf1.h:46:18:46:20 | ull | This argument should be of type 'unsigned int' but is of type 'unsigned long long' | -| printf1.h:47:19:47:21 | ull | This argument should be of type 'unsigned int' but is of type 'unsigned long long' | | printf1.h:71:19:71:20 | st | This argument should be of type 'ssize_t' but is of type 'unsigned long long' | | printf1.h:72:19:72:20 | ST | This argument should be of type 'ssize_t' but is of type 'unsigned long long' | | printf1.h:73:19:73:22 | c_st | This argument should be of type 'ssize_t' but is of type 'unsigned long long' | diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Microsoft/printf1.h b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Microsoft/printf1.h index 499d543da273..f77e0ac87fb0 100644 --- a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Microsoft/printf1.h +++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Microsoft/printf1.h @@ -44,7 +44,7 @@ void f(char *s, int i, unsigned char *us, const char *cs, signed char *ss, char printf("%d", ull); // not ok (unsigned long long -> int) printf("%u", ull); // not ok (unsigned long long -> unsigned int) printf("%x", ull); // not ok (unsigned long long -> unsigned int) - printf("%Lx", ull); // not ok (unsigned long long -> unsigned int) + printf("%Lx", ull); // ok printf("%llx", ull); // ok } diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Microsoft_no_wchar/WrongTypeFormatArguments.expected b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Microsoft_no_wchar/WrongTypeFormatArguments.expected index 64c9bd0de7c8..75536814cade 100644 --- a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Microsoft_no_wchar/WrongTypeFormatArguments.expected +++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Microsoft_no_wchar/WrongTypeFormatArguments.expected @@ -10,7 +10,6 @@ | printf1.h:44:18:44:20 | ull | This argument should be of type 'int' but is of type 'unsigned long long' | | printf1.h:45:18:45:20 | ull | This argument should be of type 'unsigned int' but is of type 'unsigned long long' | | printf1.h:46:18:46:20 | ull | This argument should be of type 'unsigned int' but is of type 'unsigned long long' | -| printf1.h:47:19:47:21 | ull | This argument should be of type 'unsigned int' but is of type 'unsigned long long' | | printf1.h:71:19:71:20 | st | This argument should be of type 'ssize_t' but is of type 'unsigned long long' | | printf1.h:72:19:72:20 | ST | This argument should be of type 'ssize_t' but is of type 'unsigned long long' | | printf1.h:73:19:73:22 | c_st | This argument should be of type 'ssize_t' but is of type 'unsigned long long' | diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Microsoft_no_wchar/printf1.h b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Microsoft_no_wchar/printf1.h index c41719ff3c63..ebff460c421f 100644 --- a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Microsoft_no_wchar/printf1.h +++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Microsoft_no_wchar/printf1.h @@ -44,7 +44,7 @@ void f(char *s, int i, unsigned char *us, const char *cs, signed char *ss, char printf("%d", ull); // not ok (unsigned long long -> int) printf("%u", ull); // not ok (unsigned long long -> unsigned int) printf("%x", ull); // not ok (unsigned long long -> unsigned int) - printf("%Lx", ull); // not ok (unsigned long long -> unsigned int) + printf("%Lx", ull); // ok printf("%llx", ull); // ok } From aa21db3ed39bba4d7d2d7a623574f0f76ea27b33 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Wed, 3 Apr 2019 11:57:38 +0100 Subject: [PATCH 3/3] CPP: Change note. --- change-notes/1.21/analysis-cpp.md | 1 + 1 file changed, 1 insertion(+) diff --git a/change-notes/1.21/analysis-cpp.md b/change-notes/1.21/analysis-cpp.md index 733a8eb9531f..762e4129e4bb 100644 --- a/change-notes/1.21/analysis-cpp.md +++ b/change-notes/1.21/analysis-cpp.md @@ -18,5 +18,6 @@ | Resource not released in destructor (`cpp/resource-not-released-in-destructor`) | Fewer false positive results | Resource allocation and deallocation functions are now determined more accurately. | | Comparison result is always the same | Fewer false positive results | The range analysis library is now more conservative about floating point values being possibly `NaN` | | Wrong type of arguments to formatting function (`cpp/wrong-type-format-argument`) | More correct results and fewer false positive results | This query now more accurately identifies wide and non-wide string/character format arguments on different platforms. Platform detection has also been made more accurate for the purposes of this query. | +| Wrong type of arguments to formatting function (`cpp/wrong-type-format-argument`) | Fewer false positive results | Non-standard uses of %L are now understood. | ## Changes to QL libraries