diff --git a/change-notes/1.19/analysis-cpp.md b/change-notes/1.19/analysis-cpp.md index 9bc73fda28c4..b1f84b60e51e 100644 --- a/change-notes/1.19/analysis-cpp.md +++ b/change-notes/1.19/analysis-cpp.md @@ -13,8 +13,8 @@ | **Query** | **Expected impact** | **Change** | |----------------------------|------------------------|------------------------------------------------------------------| | Resource not released in destructor | Fewer false positive results | Placement new is now excluded from the query. | -| Wrong type of arguments to formatting function | Fewer false positive results | False positive results involving typedefs have been removed. | | Missing return statement (`cpp/missing-return`) | Visible by default | The precision of this query has been increased from 'medium' to 'high', which makes it visible by default in LGTM. It was 'medium' in release 1.17 and 1.18 because it had false positives due to an extractor bug that was fixed in 1.18. | +| Wrong type of arguments to formatting function | Fewer false positive results | False positive results involving typedefs have been removed. Expected argument types are determined more accurately, especially for wide string and pointer types. Custom (non-standard) formatting functions are also identified more accurately. | ## Changes to QL libraries diff --git a/cpp/ql/src/Likely Bugs/Format/WrongTypeFormatArguments.ql b/cpp/ql/src/Likely Bugs/Format/WrongTypeFormatArguments.ql index 984642d109b9..4f6c139b74be 100644 --- a/cpp/ql/src/Likely Bugs/Format/WrongTypeFormatArguments.ql +++ b/cpp/ql/src/Likely Bugs/Format/WrongTypeFormatArguments.ql @@ -25,7 +25,8 @@ private predicate formattingFunctionCallExpectedType(FormattingFunctionCall ffc, ffc.getTarget() = f and f.getFormatParameterIndex() = i and ffc.getArgument(i) = fl and - fl.getConversionType(pos) = expected + fl.getConversionType(pos) = expected and + count(fl.getConversionType(pos)) = 1 ) } diff --git a/cpp/ql/src/semmle/code/cpp/commons/Printf.qll b/cpp/ql/src/semmle/code/cpp/commons/Printf.qll index 5125c972f4bb..eb9c43f5b5bb 100644 --- a/cpp/ql/src/semmle/code/cpp/commons/Printf.qll +++ b/cpp/ql/src/semmle/code/cpp/commons/Printf.qll @@ -220,6 +220,32 @@ class FormatLiteral extends Literal { getUse().getTarget().(FormattingFunction).isWideCharDefault() } + /** + * Gets the default character type expected for `%s` by this format literal. Typically + * `char` or `wchar_t`. + */ + Type getDefaultCharType() { + result = getUse().getTarget().(FormattingFunction).getDefaultCharType() + } + + /** + * Gets the non-default character type expected for `%S` by this format literal. Typically + * `wchar_t` or `char`. On some snapshots there may be multiple results where we can't tell + * which is correct for a particular function. + */ + Type getNonDefaultCharType() { + result = getUse().getTarget().(FormattingFunction).getNonDefaultCharType() + } + + /** + * Gets the wide character type for this format literal. This is usually `wchar_t`. On some + * snapshots there may be multiple results where we can't tell which is correct for a + * particular function. + */ + Type getWideCharType() { + result = getUse().getTarget().(FormattingFunction).getWideCharType() + } + /** * Holds if this `FormatLiteral` is in a context that supports * Microsoft rules and extensions. @@ -629,7 +655,6 @@ class FormatLiteral extends Literal { result = getConversionType2(n) or result = getConversionType3(n) or result = getConversionType4(n) or - result = getConversionType5(n) or result = getConversionType6(n) or result = getConversionType7(n) or result = getConversionType8(n) or @@ -696,33 +721,35 @@ class FormatLiteral extends Literal { } /** - * Gets the 'effective' string type character, that is, 's' (meaning a char string) or - * 'S' (meaning a wide string). - * - in the base case this is the same as the format type character. - * - for a `wprintf` or similar function call, the meanings are reversed. - * - the size prefixes 'l'/'w' (long) and 'h' (short) override the - * type character to effectively 'S' or 's' respectively. - */ - private string getEffectiveStringConversionChar(int n) { - exists(string len, string conv | this.parseConvSpec(n, _, _, _, _, _, len, conv) and (conv = "s" or conv = "S") | - (len = "l" and result = "S") or - (len = "w" and result = "S") or - (len = "h" and result = "s") or - (len != "l" and len != "w" and len != "h" and (result = "s" or result = "S") and (if isWideCharDefault() then result != conv else result = conv)) - ) - } - + * Gets the string type required by the nth conversion specifier. + * - in the base case this is the default for the formatting function + * (e.g. `char` for `printf`, `wchar_t` for `wprintf`). + * - the `%S` format character reverses wideness. + * - the size prefixes 'l'/'w' and 'h' override the type character + * to wide or single-byte characters respectively. + */ private Type getConversionType4(int n) { - exists(string cnv, CharType t | cnv = this.getEffectiveStringConversionChar(n) | - cnv="s" and t = result.(PointerType).getBaseType() - and not t.isExplicitlySigned() - and not t.isExplicitlyUnsigned() - ) - } - - private Type getConversionType5(int n) { - exists(string cnv | cnv = this.getEffectiveStringConversionChar(n) | - cnv="S" and result.(PointerType).getBaseType().hasName("wchar_t") + exists(string len, string conv | + this.parseConvSpec(n, _, _, _, _, _, len, conv) and + ( + ( + (conv = "s" or conv = "S") and + len = "h" and + result.(PointerType).getBaseType() instanceof PlainCharType + ) or ( + (conv = "s" or conv = "S") and + (len = "l" or len = "w") and + result.(PointerType).getBaseType() = getWideCharType() + ) or ( + conv = "s" and + (len != "l" and len != "w" and len != "h") and + result.(PointerType).getBaseType() = getDefaultCharType() + ) or ( + conv = "S" and + (len != "l" and len != "w" and len != "h") and + result.(PointerType).getBaseType() = getNonDefaultCharType() + ) + ) ) } diff --git a/cpp/ql/src/semmle/code/cpp/models/implementations/Printf.qll b/cpp/ql/src/semmle/code/cpp/models/implementations/Printf.qll index 06c6a86e4a10..ef696a526f79 100644 --- a/cpp/ql/src/semmle/code/cpp/models/implementations/Printf.qll +++ b/cpp/ql/src/semmle/code/cpp/models/implementations/Printf.qll @@ -12,7 +12,8 @@ class Printf extends FormattingFunction { hasGlobalName("wprintf") or hasGlobalName("wprintf_s") or hasGlobalName("g_printf") - ) + ) and + not exists(getDefinition().getFile().getRelativePath()) } override int getFormatParameterIndex() { result=0 } @@ -26,7 +27,15 @@ class Printf extends FormattingFunction { * The standard functions `fprintf`, `fwprintf` and their glib variants. */ class Fprintf extends FormattingFunction { - Fprintf() { this instanceof TopLevelFunction and (hasGlobalName("fprintf") or hasGlobalName("fwprintf") or hasGlobalName("g_fprintf"))} + Fprintf() { + this instanceof TopLevelFunction and + ( + hasGlobalName("fprintf") or + hasGlobalName("fwprintf") or + hasGlobalName("g_fprintf") + ) and + not exists(getDefinition().getFile().getRelativePath()) + } override int getFormatParameterIndex() { result=1 } override predicate isWideCharDefault() { hasGlobalName("fwprintf") } @@ -47,7 +56,8 @@ class Sprintf extends FormattingFunction { hasGlobalName("g_strdup_printf") or hasGlobalName("g_sprintf") or hasGlobalName("__builtin___sprintf_chk") - ) + ) and + not exists(getDefinition().getFile().getRelativePath()) } override predicate isWideCharDefault() { @@ -100,7 +110,8 @@ class Snprintf extends FormattingFunction { or hasGlobalName("g_snprintf") or hasGlobalName("wnsprintf") or hasGlobalName("__builtin___snprintf_chk") - ) + ) and + not exists(getDefinition().getFile().getRelativePath()) } override int getFormatParameterIndex() { @@ -133,10 +144,13 @@ class Snprintf extends FormattingFunction { * in the buffer. */ predicate returnsFullFormatLength() { - hasGlobalName("snprintf") or - hasGlobalName("g_snprintf") or - hasGlobalName("__builtin___snprintf_chk") or - hasGlobalName("snprintf_s") + ( + hasGlobalName("snprintf") or + hasGlobalName("g_snprintf") or + hasGlobalName("__builtin___snprintf_chk") or + hasGlobalName("snprintf_s") + ) and + not exists(getDefinition().getFile().getRelativePath()) } override int getSizeParameterIndex() { @@ -158,7 +172,8 @@ class StringCchPrintf extends FormattingFunction { or hasGlobalName("StringCbPrintfEx") or hasGlobalName("StringCbPrintf_l") or hasGlobalName("StringCbPrintf_lEx") - ) + ) and + not exists(getDefinition().getFile().getRelativePath()) } override int getFormatParameterIndex() { @@ -187,7 +202,8 @@ class Syslog extends FormattingFunction { Syslog() { this instanceof TopLevelFunction and ( hasGlobalName("syslog") - ) + ) and + not exists(getDefinition().getFile().getRelativePath()) } override int getFormatParameterIndex() { result=1 } diff --git a/cpp/ql/src/semmle/code/cpp/models/interfaces/FormattingFunction.qll b/cpp/ql/src/semmle/code/cpp/models/interfaces/FormattingFunction.qll index 2712348d880d..757012290e24 100644 --- a/cpp/ql/src/semmle/code/cpp/models/interfaces/FormattingFunction.qll +++ b/cpp/ql/src/semmle/code/cpp/models/interfaces/FormattingFunction.qll @@ -7,6 +7,38 @@ */ import semmle.code.cpp.Function + +private Type stripTopLevelSpecifiersOnly(Type t) { + ( + result = stripTopLevelSpecifiersOnly(t.(SpecifiedType).getBaseType()) + ) or ( + result = t and + not t instanceof SpecifiedType + ) +} + +/** + * A type that is used as a format string by any formatting function. + */ +Type getAFormatterWideType() { + exists(FormattingFunction ff | + result = stripTopLevelSpecifiersOnly(ff.getDefaultCharType()) and + result.getSize() != 1 + ) +} + +/** + * A type that is used as a format string by any formatting function, or `wchar_t` if + * there is none. + */ +private Type getAFormatterWideTypeOrDefault() { + result = getAFormatterWideType() or + ( + not exists(getAFormatterWideType()) and + result instanceof Wchar_t + ) +} + /** * A standard library function that uses a `printf`-like formatting string. */ @@ -20,6 +52,43 @@ abstract class FormattingFunction extends Function { */ predicate isWideCharDefault() { none() } + /** + * Gets the default character type expected for `%s` by this function. Typically + * `char` or `wchar_t`. + */ + Type getDefaultCharType() { + result = stripTopLevelSpecifiersOnly(getParameter(getFormatParameterIndex()).getType(). + getUnderlyingType().(PointerType).getBaseType()) + } + + /** + * Gets the non-default character type expected for `%S` by this function. Typically + * `wchar_t` or `char`. On some snapshots there may be multiple results where we can't tell + * which is correct for a particular function. + */ + Type getNonDefaultCharType() { + ( + getDefaultCharType().getSize() = 1 and + result = getAFormatterWideTypeOrDefault() + ) or ( + getDefaultCharType().getSize() > 1 and + result instanceof PlainCharType + ) + } + + /** + * Gets the wide character type for this function. This is usually `wchar_t`. On some + * snapshots there may be multiple results where we can't tell which is correct for a + * particular function. + */ + Type getWideCharType() { + ( + result = getDefaultCharType() or + result = getNonDefaultCharType() + ) and + result.getSize() > 1 + } + /** * Gets the position at which the output parameter, if any, occurs. */ diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongNumberOfFormatArguments/TooManyFormatArguments.expected b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongNumberOfFormatArguments/TooManyFormatArguments.expected index 3c94ae894346..0d9d84285f16 100644 --- a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongNumberOfFormatArguments/TooManyFormatArguments.expected +++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongNumberOfFormatArguments/TooManyFormatArguments.expected @@ -1,5 +1,4 @@ | custom_printf.cpp:31:5:31:12 | call to myPrintf | Format expects 2 arguments but given 3 | -| custom_printf.cpp:44:2:44:7 | call to printf | Format expects 0 arguments but given 2 | | macros.cpp:12:2:12:31 | call to printf | Format expects 2 arguments but given 3 | | macros.cpp:16:2:16:30 | call to printf | Format expects 2 arguments but given 3 | | test.c:7:2:7:7 | call to printf | Format expects 0 arguments but given 1 | diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongNumberOfFormatArguments/WrongNumberOfFormatArguments.expected b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongNumberOfFormatArguments/WrongNumberOfFormatArguments.expected index e320187584f8..db4318a4b96b 100644 --- a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongNumberOfFormatArguments/WrongNumberOfFormatArguments.expected +++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongNumberOfFormatArguments/WrongNumberOfFormatArguments.expected @@ -1,5 +1,4 @@ | custom_printf.cpp:29:5:29:12 | call to myPrintf | Format expects 2 arguments but given 1 | -| custom_printf.cpp:45:2:45:7 | call to printf | Format expects 2 arguments but given 0 | | macros.cpp:14:2:14:37 | call to printf | Format expects 4 arguments but given 3 | | macros.cpp:21:2:21:36 | call to printf | Format expects 4 arguments but given 3 | | test.c:9:2:9:7 | call to printf | Format expects 1 arguments but given 0 | diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongNumberOfFormatArguments/custom_printf.cpp b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongNumberOfFormatArguments/custom_printf.cpp index ef7e54b468ba..b80e14e7410e 100644 --- a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongNumberOfFormatArguments/custom_printf.cpp +++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongNumberOfFormatArguments/custom_printf.cpp @@ -41,6 +41,6 @@ void test_custom_printf2() { // notTheFormat format ... printf(0, "%i %i", 100, 200); // GOOD - printf("", "%i %i", 100, 200); // GOOD [FALSE POSITIVE] - printf("%i %i", "" ); // GOOD [FALSE POSITIVE] + printf("", "%i %i", 100, 200); // GOOD + printf("%i %i", "" ); // GOOD } diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/WrongTypeFormatArguments.expected b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/WrongTypeFormatArguments.expected new file mode 100644 index 000000000000..8e50e74c9e50 --- /dev/null +++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/WrongTypeFormatArguments.expected @@ -0,0 +1,10 @@ +| tests.cpp:18:15:18:22 | Hello | This argument should be of type 'char *' but is of type 'char16_t *' | +| tests.cpp:19:15:19:22 | Hello | This argument should be of type 'char *' but is of type 'wchar_t *' | +| tests.cpp:25:17:25:23 | Hello | This argument should be of type 'wchar_t *' but is of type 'char *' | +| tests.cpp:26:17:26:24 | Hello | This argument should be of type 'wchar_t *' but is of type 'char16_t *' | +| tests.cpp:30:17:30:24 | Hello | This argument should be of type 'char *' but is of type 'char16_t *' | +| tests.cpp:31:17:31:24 | Hello | This argument should be of type 'char *' but is of type 'wchar_t *' | +| tests.cpp:33:36:33:42 | Hello | This argument should be of type 'char16_t *' but is of type 'char *' | +| tests.cpp:35:36:35:43 | Hello | This argument should be of type 'char16_t *' but is of type 'wchar_t *' | +| tests.cpp:38:36:38:43 | Hello | This argument should be of type 'char *' but is of type 'char16_t *' | +| tests.cpp:39:36:39:43 | Hello | This argument should be of type 'char *' but is of type 'wchar_t *' | diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/WrongTypeFormatArguments.qlref b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/WrongTypeFormatArguments.qlref new file mode 100644 index 000000000000..6f557ace55a5 --- /dev/null +++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/WrongTypeFormatArguments.qlref @@ -0,0 +1 @@ +Likely Bugs/Format/WrongTypeFormatArguments.ql diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/formattingFunction.expected b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/formattingFunction.expected new file mode 100644 index 000000000000..bb17832eff79 --- /dev/null +++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/formattingFunction.expected @@ -0,0 +1,3 @@ +| tests.cpp:8:5:8:10 | printf | char | char16_t, wchar_t | char16_t, wchar_t | +| tests.cpp:9:5:9:11 | wprintf | wchar_t | char | wchar_t | +| tests.cpp:10:5:10:12 | swprintf | char16_t | char | char16_t | diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/formattingFunction.ql b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/formattingFunction.ql new file mode 100644 index 000000000000..45f12aca07f2 --- /dev/null +++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/formattingFunction.ql @@ -0,0 +1,8 @@ +import cpp + +from FormattingFunction f +select + f, + concat(f.getDefaultCharType().toString(), ", "), + concat(f.getNonDefaultCharType().toString(), ", "), + concat(f.getWideCharType().toString(), ", ") diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/tests.cpp b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/tests.cpp new file mode 100644 index 000000000000..bd5353460bc3 --- /dev/null +++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/tests.cpp @@ -0,0 +1,40 @@ +/* + * Test for custom definitions of *wprintf using different types than the + * platform wide character type. + */ + +typedef unsigned int size_t; + +int printf(const char * format, ...); +int wprintf(const wchar_t * format, ...); // on wchar_t +int swprintf(char16_t * s, size_t n, const char16_t * format, ...); // on char16_t + +#define BUF_SIZE (4096) + +void tests() { + char16_t buffer[BUF_SIZE]; + + printf("%s", "Hello"); // GOOD + printf("%s", u"Hello"); // BAD: expecting char + printf("%s", L"Hello"); // BAD: expecting char + + printf("%S", "Hello"); // BAD: expecting wchar_t or char16_t [NOT DETECTED] + printf("%S", u"Hello"); // GOOD + printf("%S", L"Hello"); // GOOD + + wprintf(L"%s", "Hello"); // BAD: expecting wchar_t + wprintf(L"%s", u"Hello"); // BAD: expecting wchar_t + wprintf(L"%s", L"Hello"); // GOOD + + wprintf(L"%S", "Hello"); // GOOD + wprintf(L"%S", u"Hello"); // BAD: expecting char + wprintf(L"%S", L"Hello"); // BAD: expecting char + + swprintf(buffer, BUF_SIZE, u"%s", "Hello"); // BAD: expecting char16_t + swprintf(buffer, BUF_SIZE, u"%s", u"Hello"); // GOOD + swprintf(buffer, BUF_SIZE, u"%s", L"Hello"); // BAD: expecting char16_t + + swprintf(buffer, BUF_SIZE, u"%S", "Hello"); // GOOD + swprintf(buffer, BUF_SIZE, u"%S", u"Hello"); // BAD: expecting char + swprintf(buffer, BUF_SIZE, u"%S", L"Hello"); // BAD: expecting char +} diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_word_size/WrongTypeFormatArguments.expected b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_word_size/WrongTypeFormatArguments.expected new file mode 100644 index 000000000000..f3cddba8012b --- /dev/null +++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_word_size/WrongTypeFormatArguments.expected @@ -0,0 +1,2 @@ +| tests_32.cpp:14:16:14:23 | void_ptr | This argument should be of type 'long' but is of type 'void *' | +| tests_64.cpp:14:16:14:23 | void_ptr | This argument should be of type 'long' but is of type 'void *' | diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_word_size/WrongTypeFormatArguments.qlref b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_word_size/WrongTypeFormatArguments.qlref new file mode 100644 index 000000000000..6f557ace55a5 --- /dev/null +++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_word_size/WrongTypeFormatArguments.qlref @@ -0,0 +1 @@ +Likely Bugs/Format/WrongTypeFormatArguments.ql diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_word_size/tests_32.cpp b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_word_size/tests_32.cpp new file mode 100644 index 000000000000..94aa803c6402 --- /dev/null +++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_word_size/tests_32.cpp @@ -0,0 +1,17 @@ +// semmle-extractor-options: --edg --target --edg linux_i686 +/* + * Test for printf in a snapshot that contains multiple word/pointer sizes. + */ + +int printf(const char * format, ...); + +void test_32() +{ + long l; + void *void_ptr; + + printf("%li", l); // GOOD + printf("%li", void_ptr); // BAD + printf("%p", l); // BAD [NOT DETECTED] + printf("%p", void_ptr); // GOOD +} diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_word_size/tests_64.cpp b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_word_size/tests_64.cpp new file mode 100644 index 000000000000..a9eda51c5c70 --- /dev/null +++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_word_size/tests_64.cpp @@ -0,0 +1,17 @@ +// semmle-extractor-options: --edg --target --edg linux_x86_64 +/* + * Test for printf in a snapshot that contains multiple word/pointer sizes. + */ + +int printf(const char * format, ...); + +void test_64() +{ + long l; + void *void_ptr; + + printf("%li", l); // GOOD + printf("%li", void_ptr); // BAD + printf("%p", l); // BAD [NOT DETECTED] + printf("%p", void_ptr); // GOOD +} diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_two_byte_wprintf/WrongTypeFormatArguments.expected b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_two_byte_wprintf/WrongTypeFormatArguments.expected new file mode 100644 index 000000000000..420a97ac72f1 --- /dev/null +++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_two_byte_wprintf/WrongTypeFormatArguments.expected @@ -0,0 +1,2 @@ +| printf.cpp:45:29:45:35 | test | This argument should be of type 'char *' but is of type 'char16_t *' | +| printf.cpp:52:29:52:35 | test | This argument should be of type 'char16_t *' but is of type 'wchar_t *' | diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_two_byte_wprintf/WrongTypeFormatArguments.qlref b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_two_byte_wprintf/WrongTypeFormatArguments.qlref new file mode 100644 index 000000000000..6f557ace55a5 --- /dev/null +++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_two_byte_wprintf/WrongTypeFormatArguments.qlref @@ -0,0 +1 @@ +Likely Bugs/Format/WrongTypeFormatArguments.ql diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_two_byte_wprintf/formattingFunction.expected b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_two_byte_wprintf/formattingFunction.expected new file mode 100644 index 000000000000..01f6a5ed7530 --- /dev/null +++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_two_byte_wprintf/formattingFunction.expected @@ -0,0 +1,2 @@ +| printf.cpp:15:5:15:12 | swprintf | char16_t | char | char16_t | +| printf.cpp:26:5:26:11 | sprintf | char | char16_t | char16_t | diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_two_byte_wprintf/formattingFunction.ql b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_two_byte_wprintf/formattingFunction.ql new file mode 100644 index 000000000000..45f12aca07f2 --- /dev/null +++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_two_byte_wprintf/formattingFunction.ql @@ -0,0 +1,8 @@ +import cpp + +from FormattingFunction f +select + f, + concat(f.getDefaultCharType().toString(), ", "), + concat(f.getNonDefaultCharType().toString(), ", "), + concat(f.getWideCharType().toString(), ", ") diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_two_byte_wprintf/printf.cpp b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_two_byte_wprintf/printf.cpp new file mode 100644 index 000000000000..6e0a7d3b383b --- /dev/null +++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_two_byte_wprintf/printf.cpp @@ -0,0 +1,53 @@ +/* + * Test for custom definitions of *wprintf using different types than the + * platform wide character type. + */ + +#define WCHAR char16_t +typedef void *va_list; +#define va_start(va, other) +#define va_end(args) + +int vswprintf(WCHAR *dest, WCHAR *format, va_list args) { + return 0; +} + +int swprintf(WCHAR *dest, WCHAR *format, ...) { + va_list args; + va_start(args, format); + + int ret = vswprintf(dest, format, args); + + va_end(args); + + return ret; +} + +int sprintf(char *dest, char *format, ...); + +// --- + +void test1() { + WCHAR string[20]; + + swprintf(string, u"test %s", u"test"); // GOOD +} + +void test2() { + char string[20]; + + sprintf(string, "test %S", u"test"); // GOOD +} + +void test3() { + char string[20]; + + sprintf(string, "test %s", u"test"); // BAD: `char16_t` string parameter read as `char` string +} + + +void test4() { + char string[20]; + + sprintf(string, "test %S", L"test"); // BAD: `wchar_t` string parameter read as `char16_t` string +} diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_unsigned_chars/formattingFunction.expected b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_unsigned_chars/formattingFunction.expected new file mode 100644 index 000000000000..75906fa0aed6 --- /dev/null +++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_unsigned_chars/formattingFunction.expected @@ -0,0 +1,4 @@ +| common.h:12:12:12:17 | printf | char | wchar_t | wchar_t | +| format.h:4:13:4:17 | error | char | wchar_t | wchar_t | +| real_world.h:8:12:8:18 | fprintf | char | wchar_t | wchar_t | +| real_world.h:33:6:33:12 | msg_out | char | wchar_t | wchar_t | diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_unsigned_chars/formattingFunction.ql b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_unsigned_chars/formattingFunction.ql new file mode 100644 index 000000000000..45f12aca07f2 --- /dev/null +++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_unsigned_chars/formattingFunction.ql @@ -0,0 +1,8 @@ +import cpp + +from FormattingFunction f +select + f, + concat(f.getDefaultCharType().toString(), ", "), + concat(f.getNonDefaultCharType().toString(), ", "), + concat(f.getWideCharType().toString(), ", ") 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 fa245ce45bf5..64c9bd0de7c8 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 @@ -17,7 +17,7 @@ | printf1.h:74:19:74:22 | C_ST | This argument should be of type 'ssize_t' but is of type 'unsigned long long' | | printf1.h:75:19:75:28 | sizeof() | This argument should be of type 'ssize_t' but is of type 'unsigned long long' | | printf1.h:84:23:84:35 | ... - ... | This argument should be of type 'ssize_t' but is of type 'long long' | -| real_world.h:46:36:46:43 | filename | This argument should be of type 'wchar_t *' but is of type 'char16_t *' | +| printf1.h:130:18:130:18 | 0 | This argument should be of type 'void *' but is of type 'int' | | 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/Microsoft_no_wchar/printf1.h b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Microsoft_no_wchar/printf1.h index 3907977df3f0..c41719ff3c63 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 @@ -101,3 +101,31 @@ void fun1(unsigned char* a, unsigned char* b) { printf("%td\n", pdt); // GOOD printf("%td\n", a-b); // GOOD } + +typedef wchar_t WCHAR_T; // WCHAR_T -> wchar_t -> int +typedef int MYCHAR; // MYCHAR -> int (notably not via the wchar_t typedef) + +void fun2() { + wchar_t *myString1; + WCHAR_T *myString2; + int *myString3; + MYCHAR *myString4; + + printf("%S", myString1); // GOOD + printf("%S", myString2); // GOOD + printf("%S", myString3); // GOOD + printf("%S", myString4); // GOOD +} + +typedef void *VOIDPTR; +typedef int (*FUNPTR)(int); + +void fun3(void *p1, VOIDPTR p2, FUNPTR p3, char *p4) +{ + printf("%p\n", p1); // GOOD + printf("%p\n", p2); // GOOD + printf("%p\n", p3); // GOOD + printf("%p\n", p4); // GOOD + printf("%p\n", p4 + 1); // GOOD + printf("%p\n", 0); // GOOD [FALSE POSITIVE] +}