github / github/codeql

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

未关闭
#15,574 11 条评论 0 个 reaction 已指派 0 人 在 GitHub 查看
false-positive Java
主要语言
CodeQL
星标
10.1k
派生
2.1k
平均合并
2 天 15 小时
30 天内合并 PR
141

描述

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!

贡献指南

打开贡献指南

评估

这个 Issue 还没有评估数据。

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。