C++: Reduce FPs in cpp/wrong-type-format-argument due to extraction errors

cpp/ql/lib/change-notes/2014-10-16-implicitly-declared-fns.md

@@ -0,0 +1,4 @@
+---
+category: feature
+---
+* Added the predicate `mayBeFromImplicitlyDeclaredFunction()` to the `Expr` class to represent expressions that may be the return value of an implicitly declared C function.

cpp/ql/lib/semmle/code/cpp/exprs/Call.qll

@@ -300,6 +300,11 @@ class FunctionCall extends Call, @funbindexpr {
     this.isVirtual() or
     this.getTarget().getAnAttribute().getName() = "weak"
   }
+
+  override predicate mayBeFromImplicitlyDeclaredFunction() {
+    this.getType() instanceof IntType and
+    this.getTarget().getADeclarationEntry().isImplicit()
+  }
 }
 
 /** A _user-defined_ unary `operator*` function. */

cpp/ql/lib/semmle/code/cpp/exprs/Expr.qll

@@ -534,6 +534,9 @@ class Expr extends StmtParent, @expr {
 
   /** Gets the function containing this control-flow node. */
   override Function getControlFlowScope() { result = this.getEnclosingFunction() }
+
+  /** Holds if this expression could be the return value of an implicitly declared function. */
+  predicate mayBeFromImplicitlyDeclaredFunction() { none() }
 }
 
 /**

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()
   }
 }
 

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()
     )

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.mayBeFromImplicitlyDeclaredFunction()
 select arg,
   "This format specifier for type '" + expected.getName() + "' does not match the argument type '" +
     actual.getUnspecifiedType().getName() + "'."

cpp/ql/src/change-notes/2024-10-15-wrong-type-format-argument.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Fixed false positives in the `cpp/wrong-type-format-argument` ("Wrong type of arguments to formatting function") query if there are extraction errors in the function.

cpp/ql/test/library-tests/exprs/implicitly_declared/error_type.expected

@@ -0,0 +1 @@
+| file://:0:0:0:0 | <error expr> |

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

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 |

cpp/ql/test/library-tests/exprs/implicitly_declared/errors.ql

@@ -0,0 +1,4 @@
+import cpp
+
+from Diagnostic d
+select d

cpp/ql/test/library-tests/exprs/implicitly_declared/implicit.c

@@ -0,0 +1,4 @@
+void f() {
+    f();
+    g();
+}

cpp/ql/test/library-tests/exprs/implicitly_declared/implicit.cpp

@@ -0,0 +1,6 @@
+// semmle-extractor-options: --expect_errors
+
+void f() {
+    f();
+    g();
+}

cpp/ql/test/library-tests/exprs/implicitly_declared/implicit.expected

@@ -0,0 +1 @@
+| implicit.c:3:5:3:5 | call to g |

cpp/ql/test/library-tests/exprs/implicitly_declared/implicit.ql

@@ -0,0 +1,5 @@
+import cpp
+
+from Expr e
+where e.mayBeFromImplicitlyDeclaredFunction()
+select e

cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Buildless/WrongTypeFormatArguments.expected

@@ -0,0 +1 @@
+| tests.c:6:18:6:18 | 1 | This format specifier for type 'char *' does not match the argument type 'int'. |

cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Buildless/WrongTypeFormatArguments.qlref

@@ -0,0 +1 @@
+Likely Bugs/Format/WrongTypeFormatArguments.ql

cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Buildless/tests.c

@@ -0,0 +1,10 @@
+// semmle-extractor-options: --expect_errors
+
+int printf(const char * format, ...);
+
+void f() {
+    printf("%s", 1); // BAD
+    printf("%s", implicit_function()); // GOOD - we should ignore the type
+    sprintf(0, "%s", ""); // GOOD
+    fprintf(0, "%s", ""); // GOOD
+}