diff --git a/cpp/ql/lib/change-notes/2014-10-16-implicitly-declared-fns.md b/cpp/ql/lib/change-notes/2014-10-16-implicitly-declared-fns.md new file mode 100644 index 000000000000..3a40cef48485 --- /dev/null +++ b/cpp/ql/lib/change-notes/2014-10-16-implicitly-declared-fns.md @@ -0,0 +1,5 @@ +--- +category: feature +--- +* Added the predicate `mayBeFromImplicitlyDeclaredFunction()` to the `Call` class to represent calls that may be the return value of an implicitly declared C function. +* Added the predicate `getAnExplicitDeclarationEntry()` to the `Function` class to get a `FunctionDeclarationEntry` that is not implicit. diff --git a/cpp/ql/lib/semmle/code/cpp/Function.qll b/cpp/ql/lib/semmle/code/cpp/Function.qll index 7cfa779338f7..18b7c21dbe23 100644 --- a/cpp/ql/lib/semmle/code/cpp/Function.qll +++ b/cpp/ql/lib/semmle/code/cpp/Function.qll @@ -230,6 +230,14 @@ class Function extends Declaration, ControlFlowNode, AccessHolder, @function { ) } + /** + * Gets a non-implicit function declaration entry. + */ + FunctionDeclarationEntry getAnExplicitDeclarationEntry() { + result = this.getADeclarationEntry() and + not result.isImplicit() + } + private predicate declEntry(FunctionDeclarationEntry fde) { fun_decls(unresolveElement(fde), underlyingElement(this), _, _, _) and // If one .cpp file specializes a function, and another calls the diff --git a/cpp/ql/lib/semmle/code/cpp/exprs/Call.qll b/cpp/ql/lib/semmle/code/cpp/exprs/Call.qll index 332cda770bbb..03c3e8a33714 100644 --- a/cpp/ql/lib/semmle/code/cpp/exprs/Call.qll +++ b/cpp/ql/lib/semmle/code/cpp/exprs/Call.qll @@ -149,6 +149,11 @@ class Call extends Expr, NameQualifiableElement, TCall { variableAddressEscapesTreeNonConst(va, this.getQualifier().getFullyConverted()) and i = -1 } + + /** Holds if this expression could be the return value of an implicitly declared function. */ + predicate mayBeFromImplicitlyDeclaredFunction() { + this.getTarget().getADeclarationEntry().isImplicit() + } } /** 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 7286552e3ee0..9c3bfb4f35ec 100644 --- a/cpp/ql/lib/semmle/code/cpp/models/implementations/Printf.qll +++ b/cpp/ql/lib/semmle/code/cpp/models/implementations/Printf.qll @@ -91,7 +91,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 = super.getFirstFormatArgumentIndex() } } 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 8ae48181ae18..ce65a65319a8 100644 --- a/cpp/ql/lib/semmle/code/cpp/models/interfaces/FormattingFunction.qll +++ b/cpp/ql/lib/semmle/code/cpp/models/interfaces/FormattingFunction.qll @@ -42,6 +42,21 @@ private Type getAFormatterWideTypeOrDefault() { * A standard library function that uses a `printf`-like formatting string. */ abstract class FormattingFunction extends ArrayFunction, TaintFunction { + int firstFormatArgumentIndex; + + FormattingFunction() { + firstFormatArgumentIndex > 0 and + if this.hasDefinition() + then firstFormatArgumentIndex = this.getDefinition().getNumberOfParameters() + else + if this instanceof BuiltInFunction + then firstFormatArgumentIndex = this.getNumberOfParameters() + else + forex(FunctionDeclarationEntry fde | fde = this.getAnExplicitDeclarationEntry() | + firstFormatArgumentIndex = fde.getNumberOfParameters() + ) + } + /** Gets the position at which the format parameter occurs. */ abstract int getFormatParameterIndex(); @@ -121,33 +136,7 @@ abstract class FormattingFunction extends ArrayFunction, TaintFunction { * the first format specifier in the format string. We ignore all * implicit function definitions. */ - int getFirstFormatArgumentIndex() { - // The formatting function either has a definition in the snapshot, or all - // `DeclarationEntry`s agree on the number of parameters (otherwise we don't - // really know the correct number) - if this.hasDefinition() - then result = this.getDefinition().getNumberOfParameters() - else result = this.getNumberOfExplicitParameters() - } - - /** - * Gets a non-implicit function declaration entry. - */ - private FunctionDeclarationEntry getAnExplicitDeclarationEntry() { - result = this.getADeclarationEntry() and - not result.isImplicit() - } - - /** - * Gets the number of parameters, excluding any parameters that have been defined - * from implicit function declarations. If there is some inconsistency in the number - * of parameters, then don't return anything. - */ - private int getNumberOfExplicitParameters() { - forex(FunctionDeclarationEntry fde | fde = this.getAnExplicitDeclarationEntry() | - result = fde.getNumberOfParameters() - ) - } + int getFirstFormatArgumentIndex() { result = firstFormatArgumentIndex } /** * Gets the position of the buffer size argument, if any. diff --git a/cpp/ql/src/Likely Bugs/Format/WrongTypeFormatArguments.ql b/cpp/ql/src/Likely Bugs/Format/WrongTypeFormatArguments.ql index e38bfbf562be..027f4caa8ae4 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).mayBeFromImplicitlyDeclaredFunction() select arg, "This format specifier for type '" + expected.getName() + "' does not match the argument type '" + actual.getUnspecifiedType().getName() + "'." diff --git a/cpp/ql/src/change-notes/2024-10-15-wrong-type-format-argument.md b/cpp/ql/src/change-notes/2024-10-15-wrong-type-format-argument.md new file mode 100644 index 000000000000..a7a85535d346 --- /dev/null +++ b/cpp/ql/src/change-notes/2024-10-15-wrong-type-format-argument.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* Remove results from the `cpp/wrong-type-format-argument` ("Wrong type of arguments to formatting function") query if the argument is the return value of an implicitly declared function. diff --git a/cpp/ql/test/library-tests/exprs/implicitly_declared/error_type.expected b/cpp/ql/test/library-tests/exprs/implicitly_declared/error_type.expected new file mode 100644 index 000000000000..b7974b1b2623 --- /dev/null +++ b/cpp/ql/test/library-tests/exprs/implicitly_declared/error_type.expected @@ -0,0 +1 @@ +| file://:0:0:0:0 | | diff --git a/cpp/ql/test/library-tests/exprs/implicitly_declared/error_type.ql b/cpp/ql/test/library-tests/exprs/implicitly_declared/error_type.ql new file mode 100644 index 000000000000..81e1f1fefa7c --- /dev/null +++ b/cpp/ql/test/library-tests/exprs/implicitly_declared/error_type.ql @@ -0,0 +1,5 @@ +import cpp + +from Expr e +where e.getType() instanceof ErroneousType +select e diff --git a/cpp/ql/test/library-tests/exprs/implicitly_declared/errors.expected b/cpp/ql/test/library-tests/exprs/implicitly_declared/errors.expected new file mode 100644 index 000000000000..84a1b0c7df25 --- /dev/null +++ b/cpp/ql/test/library-tests/exprs/implicitly_declared/errors.expected @@ -0,0 +1,2 @@ +| file://:0:0:0:0 | There was an error during this compilation | +| implicit.cpp:5:5:5:5 | identifier 'g' is undefined | diff --git a/cpp/ql/test/library-tests/exprs/implicitly_declared/errors.ql b/cpp/ql/test/library-tests/exprs/implicitly_declared/errors.ql new file mode 100644 index 000000000000..3fa864748e16 --- /dev/null +++ b/cpp/ql/test/library-tests/exprs/implicitly_declared/errors.ql @@ -0,0 +1,4 @@ +import cpp + +from Diagnostic d +select d diff --git a/cpp/ql/test/library-tests/exprs/implicitly_declared/functions.expected b/cpp/ql/test/library-tests/exprs/implicitly_declared/functions.expected new file mode 100644 index 000000000000..e7142fb7df2d --- /dev/null +++ b/cpp/ql/test/library-tests/exprs/implicitly_declared/functions.expected @@ -0,0 +1,6 @@ +| implicit2.c:1:7:1:7 | g | file://:0:0:0:0 | float | +| implicit2.c:1:7:1:7 | g | file://:0:0:0:0 | int | +| implicit.c:1:6:1:6 | f | file://:0:0:0:0 | void | +| implicit.c:3:5:3:5 | g | file://:0:0:0:0 | float | +| implicit.c:3:5:3:5 | g | file://:0:0:0:0 | int | +| implicit.cpp:3:6:3:6 | f | file://:0:0:0:0 | void | diff --git a/cpp/ql/test/library-tests/exprs/implicitly_declared/functions.ql b/cpp/ql/test/library-tests/exprs/implicitly_declared/functions.ql new file mode 100644 index 000000000000..71d120935a5d --- /dev/null +++ b/cpp/ql/test/library-tests/exprs/implicitly_declared/functions.ql @@ -0,0 +1,5 @@ +import cpp + +from Function fn +where fn.fromSource() +select fn, fn.getType() diff --git a/cpp/ql/test/library-tests/exprs/implicitly_declared/implicit.c b/cpp/ql/test/library-tests/exprs/implicitly_declared/implicit.c new file mode 100644 index 000000000000..5cfb42c79ba0 --- /dev/null +++ b/cpp/ql/test/library-tests/exprs/implicitly_declared/implicit.c @@ -0,0 +1,4 @@ +void f() { + f(); + g(); +} diff --git a/cpp/ql/test/library-tests/exprs/implicitly_declared/implicit.cpp b/cpp/ql/test/library-tests/exprs/implicitly_declared/implicit.cpp new file mode 100644 index 000000000000..f64bf61753fa --- /dev/null +++ b/cpp/ql/test/library-tests/exprs/implicitly_declared/implicit.cpp @@ -0,0 +1,6 @@ +// semmle-extractor-options: --expect_errors + +void f() { + f(); + g(); +} diff --git a/cpp/ql/test/library-tests/exprs/implicitly_declared/implicit.expected b/cpp/ql/test/library-tests/exprs/implicitly_declared/implicit.expected new file mode 100644 index 000000000000..8f626de5e7cc --- /dev/null +++ b/cpp/ql/test/library-tests/exprs/implicitly_declared/implicit.expected @@ -0,0 +1 @@ +| implicit.c:3:5:3:5 | call to g | diff --git a/cpp/ql/test/library-tests/exprs/implicitly_declared/implicit.ql b/cpp/ql/test/library-tests/exprs/implicitly_declared/implicit.ql new file mode 100644 index 000000000000..fe640c83de9d --- /dev/null +++ b/cpp/ql/test/library-tests/exprs/implicitly_declared/implicit.ql @@ -0,0 +1,5 @@ +import cpp + +from Call c +where c.mayBeFromImplicitlyDeclaredFunction() +select c diff --git a/cpp/ql/test/library-tests/exprs/implicitly_declared/implicit2.c b/cpp/ql/test/library-tests/exprs/implicitly_declared/implicit2.c new file mode 100644 index 000000000000..0f4c2a489e39 --- /dev/null +++ b/cpp/ql/test/library-tests/exprs/implicitly_declared/implicit2.c @@ -0,0 +1 @@ +float g(); 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 new file mode 100644 index 000000000000..745f2f790f79 --- /dev/null +++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Buildless/WrongTypeFormatArguments.expected @@ -0,0 +1 @@ +| tests.c:7:18:7:18 | 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/WrongTypeFormatArguments.qlref b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Buildless/WrongTypeFormatArguments.qlref new file mode 100644 index 000000000000..6f557ace55a5 --- /dev/null +++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Buildless/WrongTypeFormatArguments.qlref @@ -0,0 +1 @@ +Likely Bugs/Format/WrongTypeFormatArguments.ql 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 new file mode 100644 index 000000000000..81698c497c57 --- /dev/null +++ b/cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Buildless/tests.c @@ -0,0 +1,11 @@ +// semmle-extractor-options: --expect_errors + +int printf(const char * format, ...); +int fprintf(); + +void f() { + printf("%s", 1); // BAD + printf("%s", implicit_function()); // GOOD - we should ignore the type + sprintf(0, "%s", ""); // GOOD + fprintf(0, "%s", ""); // GOOD +} diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-022/semmle/tests/stdlib.h b/cpp/ql/test/query-tests/Security/CWE/CWE-022/semmle/tests/stdlib.h index 53344da57d6d..8c0cb21482be 100644 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-022/semmle/tests/stdlib.h +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-022/semmle/tests/stdlib.h @@ -17,3 +17,4 @@ void *malloc(size_t size); double strtod(const char *ptr, char **endptr); char *getenv(const char *name); ssize_t read(int fd, void *buffer, size_t count); +int snprintf(char *s, size_t n, const char *format, ...);