Go: go/log-injection produces false positives for logrus when sanitising formatters are used

[mbg]

Description of the false positive

The go/log-injection (CWE-117) query identifies log entries that are created from user input without proper sanitisation. The logrus library is vulnerable to this when the default output formatter is used. However, different output formatters, such as JSONFormatter, may sanitise log entries themselves. The go/log-injection query is not currently aware of this behaviour and will report false positives whenever logrus is used for logging and log entries are based on unsanitised user data, even if a sanitising output formatter is used.

Code samples or links to source code

In the following example, go/log-injection will report that the log entry constructed using logrus.Fields depends on a user-provided value that has not been sanitised:

func example(req *http.Request, ctx *goproxy.ProxyCtx) {
	username := req.URL.Query()["username"][0]
	logrus.SetFormatter(&logrus.JSONFormatter{})
	logrus.WithFields(logrus.Fields{
		"USERNAME": username,
	})
}

[elgohr]

Hey, is #12132 an intermediate state or the final result?

[smowton]

Most likely permanent. We found that accurately diagnosing whether a JSON or other injection-resistant formatter would be in use was too error-prone; we'd either get many false positives or many false negatives whichever way we tried to characterise the situation. Other languages already specify log-injection is a medium-precision query for similar reasons, so we figured best to simply bring the Go query into line.

[elgohr]

Would be great to have a way for ignoring a set of CWEs within a scan (like paths-ignore), so people don't have to ignore every single finding for every log-line.

[smowton]

Does https://docs.github.com/en/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/customizing-code-scanning#excluding-specific-queries-from-analysis help the case you're describing?