github / github/codeql

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

Aperta
#15,574 11 commenti 0 reazioni 0 assegnatari Vedi su GitHub
false-positive Java
Lingua principale
CodeQL
Stelle
10.1k
Fork
2.1k
Merge medio
2g 15h
PR unite (30g)
141

Descrizione

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!

Guida per i contributori

Apri la guida per i contributori

Valutazione

Questa issue non è ancora stata valutata.

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.