False positive - LogInjection (CWE 117) is not mitigated via Log4j2 %{encodeCRLF) pattern

[otap63]

Hello, I have a case where I sanitize user inputs using log4j2 Pattern rule to escape '\n' and '\r' using the encode pattern %encode as follows where the user provided messages to log4j2.log() methods are encoded seamlessly:

Log4j2.xml:

PatternLayout pattern="%d{HH:mm:ss.SSS} [%t] %-5level %logger{36} -%encode{%msg}{CRLF}%n"

Code:

log("UserId: {}", userId);
 ... 

in order to mitigate a LogInjection high vulnebarility (CWE-117) issue reported by CodeQL in Java. The problem is that CodeQL is not happy as it still reports the same set of LogInjection issues after the sanitization.

However, if I sanitize the user provided inputs in the log() messages, using the following method, CodeQL is happy.

private String escapeCRLF(String msg) {
    return (msg == null ? null : msg.replace("\n", "\\n" ).replace("\r", "\\r" ));
}
...
log("UserId: {}", escapeCRLF(userId));
...

So, apparently, CodeQL has a rule to recognize the 2nd mitigation method but somehow it misses the sanitization provided via log4j2 encoding CRLF rule, which has the exact same functionality as the above escapeCRLF method. I like the 1st solution which is uniform throughout the code base, requiring no code change. So, I am wondering if you would know how to make CodeQL happy if I deploy the Log4j2 solution.

Thanks in advance!

[aeisenberg]

Which query is giving you the alert?

[otap63]

Rule ID: java/log-injection LogInjection.ql

Thanks!

[aeisenberg]

Thanks for letting us know. This is indeed a false positive and we're tracking it internally. I can't make any promises on if or when we'll fix it, but we're aware of this and are determining how to address it. We'll update this issue when there are changes.

[otap63]

Thank you for your prompt response and effort to fix this issue. Highly appreciated!