[CPP-370] Improve handling of _ macros by using taint sanitizers.

Author: zlaski-semmle

cpp/ql/src/Likely Bugs/Format/NonConstantFormat.ql

@@ -42,14 +42,22 @@ predicate whitelistFunction(Function f, int arg) {
   (arg = 1 or arg = 2)
 }
 
-predicate whitelisted(FunctionCall fc) {
-  exists(Function f, int arg | f = fc.getTarget() | whitelistFunction(f, arg))
+// we assume that ALL uses of the `_` macro
+// return constant string literals
+predicate underscoreMacro(Expr e) {
+  exists(MacroInvocation mi |
+    mi.getMacroName() = "_" and
+    mi.getExpr() = e
+  )
 }
 
 predicate isNonConst(DataFlow::Node node) {
   exists(Expr e | e = node.asExpr() |
     exists(FunctionCall fc | fc = e.(FunctionCall) |
-      not whitelisted(fc) and not fc.getTarget().hasDefinition()
+      not (
+        whitelistFunction(fc.getTarget(), _) or
+        fc.getTarget().hasDefinition()
+      )
     )
     or
     exists(Parameter p | p = e.(VariableAccess).getTarget().(Parameter) |
@@ -97,10 +105,10 @@ class NonConstFlow extends TaintTracking::Configuration {
   override predicate isSource(DataFlow::Node source) { isNonConst(source) }
 
   override predicate isSink(DataFlow::Node sink) {
-    exists(FormattingFunctionCall fc |
-      sink.asExpr() = fc.getArgument(fc.getFormatParameterIndex())
-    )
+    exists(FormattingFunctionCall fc | sink.asExpr() = fc.getArgument(fc.getFormatParameterIndex()))
   }
+
+  override predicate isSanitizer(DataFlow::Node node) { underscoreMacro(node.asExpr()) }
 }
 
 from FormattingFunctionCall call, Expr formatString

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

@@ -31,7 +31,7 @@ int main(int argc, char **argv) {
 	else
 		printf("No argument supplied.\n"); // ok
 
-	printf(_("No argument supplied.\n")); // not ok
+	printf(_("No argument supplied.\n")); // ok
 
 	printf(dgettext(NULL, "No argument supplied.\n")); // ok
 
@@ -40,10 +40,10 @@ int main(int argc, char **argv) {
 	printf(gettext("%d arguments\n"), argc-1); // ok
 	printf(any_random_function("%d arguments\n"), argc-1); // not ok
 
-	// Since `_` is mapped to `some_random_function` above,
-	// the following call will be flagged.
+	// Even though `_` is mapped to `some_random_function` above,
+	// the following call should not  be flagged.
 	printf(_(any_random_function("%d arguments\n")),
-			argc-1); // not ok
+			argc-1); // ok
 
 	return 0;
 }

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

@@ -1,7 +1,5 @@
 | NonConstantFormat.c:30:10:30:16 | access to array | The format string argument to printf should be constant to prevent security issues and other potential errors. |
-| NonConstantFormat.c:34:9:34:36 | call to any_random_function | The format string argument to printf should be constant to prevent security issues and other potential errors. |
 | NonConstantFormat.c:41:9:41:27 | call to any_random_function | The format string argument to printf should be constant to prevent security issues and other potential errors. |
-| NonConstantFormat.c:45:9:45:48 | call to any_random_function | The format string argument to printf should be constant to prevent security issues and other potential errors. |
 | nested.cpp:21:23:21:26 | fmt0 | The format string argument to snprintf should be constant to prevent security issues and other potential errors. |
 | nested.cpp:79:32:79:38 | call to get_fmt | The format string argument to diagnostic should be constant to prevent security issues and other potential errors. |
 | nested.cpp:87:18:87:20 | fmt | The format string argument to diagnostic should be constant to prevent security issues and other potential errors. |

cpp/ql/test/query-tests/Likely Bugs/Format/NonConstantFormat/nested.cpp

@@ -18,7 +18,7 @@ extern "C" int snprintf ( char * s, int n, const char * format, ... );
 struct A {
   void do_print(const char *fmt0) {
     char buf[32];
-    snprintf(buf, 32, fmt0); // GOOD
+    snprintf(buf, 32, fmt0); // BAD [FALSE POSITIVE]
   }
 };
 
@@ -39,7 +39,7 @@ struct C {
 
 void foo(void) {
   C c;
-  c.do_some_printing(c.ext_fmt_str());
+  c.do_some_printing(c.ext_fmt_str()); // GOOD [NOT DETECTED]
 }
 
 struct some_class {
@@ -76,15 +76,15 @@ void diagnostic(const char *fmt, ...)
 }
 
 void bar(void) {
-    diagnostic (some_instance->get_fmt());  // GOOD
+    diagnostic (some_instance->get_fmt());  // BAD
 }
 
 namespace ns {
 
   class blab {
     void out1(void) {
       char *fmt = (char *)__builtin_alloca(10);
-      diagnostic(fmt);  // GOOD
+      diagnostic(fmt);  // BAD
     }
   };
 }