Adjust SafeFormatArgumentSanitizer to use-use flow

Make it sanitize the result of the call rather than the input, so that
further uses of the input are still tainted. This means that it catches
things like `log.Print(fmt.Sprintf("user %q logged in.\n", username))`
where the argument to the LoggerCall contains a StringFormatCall, but
it misses things like `log.Printf("user %q logged in.\n", username)`. So
we extract the logic into a predicate and apply it as a condition in the
sink as well.

The downside of this approach is that if there are two tainted inputs
and only one has a safe format argument then we still sanitize the
result. Hopefully this is rare.

Author: owen-mc

go/ql/lib/semmle/go/security/LogInjectionCustomizations.qll

@@ -35,7 +35,15 @@ module LogInjection {
 
   /** An argument to a logging mechanism. */
   class LoggerSink extends Sink {
-    LoggerSink() { this = any(LoggerCall log).getAValueFormattedMessageComponent() }
+    LoggerSink() {
+      exists(LoggerCall call |
+        this = call.getAValueFormattedMessageComponent() and
+        // exclude arguments to `call` which have a safe format argument, which
+        // aren't caught by SafeFormatArgumentSanitizer as that sanitizes the
+        // result of the call.
+        not safeFormatArgument(this, call)
+      )
+    }
   }
 
   /**
@@ -47,6 +55,22 @@ module LogInjection {
     ReplaceSanitizer() { this.getReplacedString() = ["\r", "\n"] }
   }
 
+  /**
+   * Holds if `arg` is an argument to `call` that is formatted using the `%q`
+   * directive. This formatting directive replaces newline characters with
+   * escape sequences, so `arg` would not be a sink for log injection.
+   */
+  private predicate safeFormatArgument(
+    DataFlow::Node arg, StringOps::Formatting::StringFormatCall call
+  ) {
+    exists(string safeDirective |
+      // Mark "%q" formats as safe, but not "%#q", which would preserve newline characters.
+      safeDirective.regexpMatch("%[^%#]*q")
+    |
+      arg = call.getOperand(_, safeDirective)
+    )
+  }
+
   /**
    * An argument that is formatted using the `%q` directive, considered as a sanitizer
    * for log injection.
@@ -55,10 +79,10 @@ module LogInjection {
    */
   private class SafeFormatArgumentSanitizer extends Sanitizer {
     SafeFormatArgumentSanitizer() {
-      exists(StringOps::Formatting::StringFormatCall call, string safeDirective |
-        this = call.getOperand(_, safeDirective) and
-        // Mark "%q" formats as safe, but not "%#q", which would preserve newline characters.
-        safeDirective.regexpMatch("%[^%#]*q")
+      exists(DataFlow::Node arg, StringOps::Formatting::StringFormatCall call |
+        safeFormatArgument(arg, call)
+      |
+        this = call.getAResult()
       )
     }
   }