From 6df8fdc23329b41a8e7f4d0a98a25ca10ee18f99 Mon Sep 17 00:00:00 2001 From: Calum Grant Date: Tue, 28 Jan 2025 14:04:33 +0000 Subject: [PATCH 1/4] C++: Add test for cpp/wrong-type-format-argument --- .../WrongTypeFormatArguments.expected | 1 + .../WrongTypeFormatArguments/Buildless/tests.c | 18 ++++++++++++++++++ .../Buildless/tests2.c | 14 ++++++++++++++ 3 files changed, 33 insertions(+) create mode 100644 cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Buildless/tests2.c diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Buildless/WrongTypeFormatArguments.expected b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Buildless/WrongTypeFormatArguments.expected index 745f2f790f79..468ae1129bdf 100644 --- a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Buildless/WrongTypeFormatArguments.expected +++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Buildless/WrongTypeFormatArguments.expected @@ -1 +1,2 @@ | tests.c:7:18:7:18 | 1 | This format specifier for type 'char *' does not match the argument type 'int'. | +| tests.c:29:27:29:27 | 1 | This format specifier for type 'char *' does not match the argument type 'int'. | diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Buildless/tests.c b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Buildless/tests.c index 175d2f23182d..c5b3d1df493a 100644 --- a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Buildless/tests.c +++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Buildless/tests.c @@ -10,3 +10,21 @@ void f(UNKNOWN_CHAR * str) { fprintf(0, "%s", ""); // GOOD printf("%s", str); // GOOD - erroneous type is ignored } + +#define va_list void* +#define va_start(x, y) x = 0; +#define va_arg(x, y) ((y)x) +#define va_end(x) +int vprintf(const char * format, va_list args); + +int my_printf(const char * format, ...) { + va_list args; + va_start(args, format); + int result = vprintf(format, args); + va_end(args); + return result; +} + +void linker_awareness_test() { + my_printf("%s%d", "", 1); // GOOD +} diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Buildless/tests2.c b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Buildless/tests2.c new file mode 100644 index 000000000000..69f5761d6a85 --- /dev/null +++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Buildless/tests2.c @@ -0,0 +1,14 @@ +#define va_list void* +#define va_start(x, y) x = 0; +#define va_arg(x, y) ((y)x) +#define va_end(x) + +int vprintf(const char * format, va_list args); + +int my_printf(void * p,const char * format, ...) { + va_list args; + va_start(args, format); + int result = vprintf(format, args); + va_end(args); + return result; +} From cc35ec49e4cbac914fbf4e943a28ad3c610bda14 Mon Sep 17 00:00:00 2001 From: Calum Grant Date: Tue, 28 Jan 2025 14:06:38 +0000 Subject: [PATCH 2/4] C++: Remove linker-awareness FPs --- cpp/ql/src/Likely Bugs/Format/WrongTypeFormatArguments.ql | 7 ++++++- .../Buildless/WrongTypeFormatArguments.expected | 1 - 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/cpp/ql/src/Likely Bugs/Format/WrongTypeFormatArguments.ql b/cpp/ql/src/Likely Bugs/Format/WrongTypeFormatArguments.ql index 272ef8369d0e..738760656b00 100644 --- a/cpp/ql/src/Likely Bugs/Format/WrongTypeFormatArguments.ql +++ b/cpp/ql/src/Likely Bugs/Format/WrongTypeFormatArguments.ql @@ -152,6 +152,10 @@ predicate trivialConversion(ExpectedType expected, Type actual) { */ int sizeof_IntType() { exists(IntType it | result = it.getSize()) } +predicate functionHasUniqueArguments(Function fn) { + forall(Parameter p | p = fn.getAParameter() | count(p.getType().getUnspecifiedType()) = 1) +} + from FormattingFunctionCall ffc, int n, Expr arg, Type expected, Type actual where ( @@ -171,7 +175,8 @@ where not arg.isAffectedByMacro() and not arg.isFromUninstantiatedTemplate(_) and not actual.stripType() instanceof ErroneousType and - not arg.(Call).mayBeFromImplicitlyDeclaredFunction() + not arg.(Call).mayBeFromImplicitlyDeclaredFunction() and + functionHasUniqueArguments(ffc.getTarget()) select arg, "This format specifier for type '" + expected.getName() + "' does not match the argument type '" + actual.getUnspecifiedType().getName() + "'." diff --git a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Buildless/WrongTypeFormatArguments.expected b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Buildless/WrongTypeFormatArguments.expected index 468ae1129bdf..745f2f790f79 100644 --- a/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Buildless/WrongTypeFormatArguments.expected +++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Buildless/WrongTypeFormatArguments.expected @@ -1,2 +1 @@ | tests.c:7:18:7:18 | 1 | This format specifier for type 'char *' does not match the argument type 'int'. | -| tests.c:29:27:29:27 | 1 | This format specifier for type 'char *' does not match the argument type 'int'. | From a033ba934755e7e80b723195362c3031cc253b2d Mon Sep 17 00:00:00 2001 From: Calum Grant Date: Wed, 29 Jan 2025 13:36:10 +0000 Subject: [PATCH 3/4] C++: Detect multiple definitions based on the format parameter index --- cpp/ql/src/Likely Bugs/Format/WrongTypeFormatArguments.ql | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/cpp/ql/src/Likely Bugs/Format/WrongTypeFormatArguments.ql b/cpp/ql/src/Likely Bugs/Format/WrongTypeFormatArguments.ql index 738760656b00..75fe855c6f91 100644 --- a/cpp/ql/src/Likely Bugs/Format/WrongTypeFormatArguments.ql +++ b/cpp/ql/src/Likely Bugs/Format/WrongTypeFormatArguments.ql @@ -152,10 +152,6 @@ predicate trivialConversion(ExpectedType expected, Type actual) { */ int sizeof_IntType() { exists(IntType it | result = it.getSize()) } -predicate functionHasUniqueArguments(Function fn) { - forall(Parameter p | p = fn.getAParameter() | count(p.getType().getUnspecifiedType()) = 1) -} - from FormattingFunctionCall ffc, int n, Expr arg, Type expected, Type actual where ( @@ -176,7 +172,8 @@ where not arg.isFromUninstantiatedTemplate(_) and not actual.stripType() instanceof ErroneousType and not arg.(Call).mayBeFromImplicitlyDeclaredFunction() and - functionHasUniqueArguments(ffc.getTarget()) + // Make sure that the format function definition is consistent + count(ffc.getTarget().getFormatParameterIndex()) = 1 select arg, "This format specifier for type '" + expected.getName() + "' does not match the argument type '" + actual.getUnspecifiedType().getName() + "'." From 6e3a169544c667d7fe4484cf727d1f9cbd29071a Mon Sep 17 00:00:00 2001 From: Calum Grant Date: Mon, 3 Feb 2025 09:48:06 +0000 Subject: [PATCH 4/4] C++: Add change note --- cpp/ql/src/change-notes/2025-01-31-format-args.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 cpp/ql/src/change-notes/2025-01-31-format-args.md diff --git a/cpp/ql/src/change-notes/2025-01-31-format-args.md b/cpp/ql/src/change-notes/2025-01-31-format-args.md new file mode 100644 index 000000000000..41f3d6bb202e --- /dev/null +++ b/cpp/ql/src/change-notes/2025-01-31-format-args.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* The "Wrong type of arguments to formatting function" query (`cpp/wrong-type-format-argument`) now produces fewer FPs if the formatting function has multiple definitions.