diff --git a/go/ql/lib/semmle/go/security/LogInjectionCustomizations.qll b/go/ql/lib/semmle/go/security/LogInjectionCustomizations.qll index 565cf29a450..4518104d33a 100644 --- a/go/ql/lib/semmle/go/security/LogInjectionCustomizations.qll +++ b/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() ) } }