From 354b62337e24eef34ab373bd227e6dc7776efa73 Mon Sep 17 00:00:00 2001 From: Calum Grant Date: Mon, 14 Oct 2024 17:10:55 +0100 Subject: [PATCH] C++: Clean up false-positives --- .../lib/semmle/code/cpp/models/implementations/Printf.qll | 6 ++++-- .../code/cpp/models/interfaces/FormattingFunction.qll | 2 +- cpp/ql/src/Likely Bugs/Format/WrongTypeFormatArguments.ql | 3 ++- .../Buildless/WrongTypeFormatArguments.expected | 3 --- .../Format/WrongTypeFormatArguments/Buildless/tests.c | 8 ++++---- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/cpp/ql/lib/semmle/code/cpp/models/implementations/Printf.qll b/cpp/ql/lib/semmle/code/cpp/models/implementations/Printf.qll index 7286552e3ee04..32498f19c80fb 100644 --- a/cpp/ql/lib/semmle/code/cpp/models/implementations/Printf.qll +++ b/cpp/ql/lib/semmle/code/cpp/models/implementations/Printf.qll @@ -50,6 +50,8 @@ private class Fprintf extends FormattingFunction, NonThrowingFunction { override int getFormatParameterIndex() { result = 1 } override int getOutputParameterIndex(boolean isStream) { result = 0 and isStream = true } + + override int getFirstFormatArgumentIndex() { result = 2 } } /** @@ -91,7 +93,7 @@ private class Sprintf extends FormattingFunction, NonThrowingFunction { override int getFirstFormatArgumentIndex() { if this.hasName("__builtin___sprintf_chk") then result = 4 - else result = this.getNumberOfParameters() + else result = this.getNumberOfExplicitParameters() } } @@ -134,7 +136,7 @@ private class SnprintfImpl extends Snprintf, AliasFunction, SideEffectFunction, result = 5 or name != "__builtin___snprintf_chk" and - result = this.getNumberOfParameters() + result = this.getNumberOfExplicitParameters() ) ) } diff --git a/cpp/ql/lib/semmle/code/cpp/models/interfaces/FormattingFunction.qll b/cpp/ql/lib/semmle/code/cpp/models/interfaces/FormattingFunction.qll index 8ae48181ae18a..28e4bfa59652c 100644 --- a/cpp/ql/lib/semmle/code/cpp/models/interfaces/FormattingFunction.qll +++ b/cpp/ql/lib/semmle/code/cpp/models/interfaces/FormattingFunction.qll @@ -143,7 +143,7 @@ abstract class FormattingFunction extends ArrayFunction, TaintFunction { * from implicit function declarations. If there is some inconsistency in the number * of parameters, then don't return anything. */ - private int getNumberOfExplicitParameters() { + int getNumberOfExplicitParameters() { forex(FunctionDeclarationEntry fde | fde = this.getAnExplicitDeclarationEntry() | result = fde.getNumberOfParameters() ) diff --git a/cpp/ql/src/Likely Bugs/Format/WrongTypeFormatArguments.ql b/cpp/ql/src/Likely Bugs/Format/WrongTypeFormatArguments.ql index e38bfbf562be2..6ec448c9e3710 100644 --- a/cpp/ql/src/Likely Bugs/Format/WrongTypeFormatArguments.ql +++ b/cpp/ql/src/Likely Bugs/Format/WrongTypeFormatArguments.ql @@ -170,7 +170,8 @@ where ) and not arg.isAffectedByMacro() and not arg.isFromUninstantiatedTemplate(_) and - not actual.getUnspecifiedType() instanceof ErroneousType + not actual.getUnspecifiedType() instanceof ErroneousType and + not arg.(Call).getTarget().getADeclarationEntry().isImplicit() 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 1890aa6b9b416..745f2f790f799 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,4 +1 @@ | tests.c:7:18:7:18 | 1 | This format specifier for type 'char *' does not match the argument type 'int'. | -| tests.c:8:18:8:34 | call to implicit_function | This format specifier for type 'char *' does not match the argument type 'int'. | -| tests.c:9:13:9:13 | 0 | This format specifier for type 'char *' does not match the argument type 'int'. | -| tests.c:10:13:10:13 | 0 | 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 6c0cf81644b38..92300d609c105 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 @@ -4,8 +4,8 @@ int printf(const char * format, ...); int fprintf(); int f() { - printf("%s", 1); // BAD - TP - printf("%s", implicit_function()); // BAD (FP) - we should not infer the return type - sprintf(0, "%s", ""); // BAD (FP) - fprintf(0, "%s", ""); // BAD (FP) + printf("%s", 1); // BAD + printf("%s", implicit_function()); // GOOD - we should not infer the return type + sprintf(0, "%s", ""); // GOOD + fprintf(0, "%s", ""); // GOOD }