java 如何修复 Veracode CWE 117（不正确的日志输出中和）

How to fix Veracode CWE 117 (Improper Output Neutralization for Logs)

提问by Vitaliy Borisok

There is an Spring global @ExceptionHandler(Exception.class)method which logs exception like that:

有一个 Spring 全局@ExceptionHandler(Exception.class)方法可以记录如下异常：

@ExceptionHandler(Exception.class)
void handleException(Exception ex) {
    logger.error("Simple error message", ex);
...

Veracode scan says that this logging has Improper Output Neutralization for Logsand suggest to use ESAPI logger. Is there any way how to fix this vulnerability without changing logger to ESAPI? This is the only place in code where I faced this issue and I try to figure out how to fix it with minimum changes. Maybe ESAPI has some methods I haven't noticed?

Veracode 扫描表明此日志记录具有Improper Output Neutralization for Logs并建议使用 ESAPI 记录器。有什么办法可以在不将记录器更改为 ESAPI 的情况下修复此漏洞？这是我在代码中遇到此问题的唯一地方，我试图找出如何以最少的更改修复它。也许 ESAPI 有一些我没有注意到的方法？

P.S. Current logger is Log4j over slf4j

PS 当前记录器是 Log4j over slf4j

UPD:In the end I used ESAPI logger. I thought it wouldn't use my default logging service, but I was wrong and it simply used my slf4j logger interface with appropriate configuration.

UPD：最后我使用了 ESAPI 记录器。我认为它不会使用我的默认日志记录服务，但我错了，它只是使用了我的 slf4j 记录器接口和适当的配置。

private static final Logger logger = ESAPI.getLogger(MyClass.class);
...
logger.error(null, "Simple error message", ex);

ESAPI has extension of log4j logger and logger factory. It can be configured what to use in ESAPI.properties. For example:

ESAPI 有 log4j logger 和 logger factory 的扩展。它可以配置在 ESAPI.properties 中使用什么。例如：

ESAPI.Logger=org.owasp.esapi.reference.Log4JLogFactory

回答by avgvstvs

Is there any way how to fix this vulnerability without changing logger to ESAPI?

有什么办法可以在不将记录器更改为 ESAPI 的情况下修复此漏洞？

In short, yes.

简而言之，是的。

TLDR:

域名注册地址：

First understand the gravity of the error. The main concern is in falsifying the log statments. Say you had code like this:

首先了解错误的严重性。主要关注的是伪造日志声明。假设你有这样的代码：

log.error( transactionId + " for user " + username + " was unsuccessful."

If either variable is under user control they can inject false logging statements by using inputs like \r\n for user foobar was successful\rnthus allowing them to falsify the log and cover their tracks. (Well, in this contrived case, just make it a little harder to see what happened.)

如果其中一个变量在用户控制之下，他们可以通过使用类似的输入来注入错误的日志语句，\r\n for user foobar was successful\rn从而允许他们伪造日志并掩盖他们的踪迹。（好吧，在这个人为的案例中，只是让它更难看到发生了什么。）

The second method of attack is more of a chess move. Many logs are HTML formatted to be viewed in another program, for this example, we'll pretend the logs are meant to be HTML files to be viewed in a browser. Now we inject <script src=”https://evilsite.com/hook.js” type=”text/javascript”></script>and you will have hooked a browser with an exploitation framework that's most likely executing as a server admin... because its doubtful that the CEO is going to be reading the log. Now the real hack can begin.

第二种攻击方法更像是棋步。许多日志是 HTML 格式的，可​​以在另一个程序中查看，对于这个例子，我们假设日志是要在浏览器中查看的 HTML 文件。现在我们注入<script src=”https://evilsite.com/hook.js” type=”text/javascript”></script>，你将使用一个漏洞利用框架钩住浏览器，该框架很可能以服务器管理员的身份执行……因为它怀疑 CEO 是否会阅读日志。现在可以开始真正的黑客攻击了。

Defenses:

防御：

A simple defense is to make sure that all log statements with userinput escape the characters '\n' and '\r' with something obvious, like '?' or you can do what ESAPI does and escape with the underscore. It really doesn't matter as long as its consistent, just keep in mind not to use character sets that would confuse you in the log. Something like userInput.replaceAll("\r", "?").replaceAll("\n", "?");

一个简单的防御是确保所有带有用户输入的日志语句都使用明显的字符转义字符 '\n' 和 '\r'，例如 '?' 或者您可以执行 ESAPI 的操作并使用下划线进行转义。只要它是一致的，这真的无关紧要，只要记住不要使用会使您在日志中混淆的字符集。就像是userInput.replaceAll("\r", "?").replaceAll("\n", "?");

I also find it useful to make sure that log formats are exquisitely specified... meaning that you make sure you have a strict standard for what log statements need to look like and construct your formatting so that catching a malicious user is easier. All programmers must submit to the party and follow the format!

我还发现确保精心指定日志格式很有用……这意味着您确保对日志语句的外观有严格的标准并构建格式，以便更容易地捕获恶意用户。所有程序员必须服从党并遵循格式！

To defend against the HTML scenario, I would use the [OWASP encoder project][1]

为了抵御 HTML 场景，我将使用 [OWASP 编码器项目][1]

As to why ESAPI's implementation is suggested, it is a very battle-tested library, but in a nutshell, this is essentially what we do. See the code:

至于为什么建议使用 ESAPI 的实现，它是一个经过实战考验的库，但简而言之，这就是我们所做的。看代码：

/**
 * Log the message after optionally encoding any special characters that might be dangerous when viewed
 * by an HTML based log viewer. Also encode any carriage returns and line feeds to prevent log
 * injection attacks. This logs all the supplied parameters plus the user ID, user's source IP, a logging
 * specific session ID, and the current date/time.
 *
 * It will only log the message if the current logging level is enabled, otherwise it will
 * discard the message.
 *
 * @param level defines the set of recognized logging levels (TRACE, INFO, DEBUG, WARNING, ERROR, FATAL)
 * @param type the type of the event (SECURITY SUCCESS, SECURITY FAILURE, EVENT SUCCESS, EVENT FAILURE)
 * @param message the message to be logged
 * @param throwable the {@code Throwable} from which to generate an exception stack trace.
 */
private void log(Level level, EventType type, String message, Throwable throwable) {

    // Check to see if we need to log.
    if (!isEnabledFor(level)) {
        return;
    }

    // ensure there's something to log
    if (message == null) {
        message = "";
    }

    // ensure no CRLF injection into logs for forging records
    String clean = message.replace('\n', '_').replace('\r', '_');
    if (ESAPI.securityConfiguration().getLogEncodingRequired()) {
        clean = ESAPI.encoder().encodeForHTML(message);
        if (!message.equals(clean)) {
            clean += " (Encoded)";
        }
    }

    // log server, port, app name, module name -- server:80/app/module
    StringBuilder appInfo = new StringBuilder();
    if (ESAPI.currentRequest() != null && logServerIP) {
        appInfo.append(ESAPI.currentRequest().getLocalAddr()).append(":").append(ESAPI.currentRequest().getLocalPort());
    }
    if (logAppName) {
        appInfo.append("/").append(applicationName);
    }
    appInfo.append("/").append(getName());

    //get the type text if it exists
    String typeInfo = "";
    if (type != null) {
        typeInfo += type + " ";
    }

    // log the message
    // Fix for https://code.google.com/p/owasp-esapi-java/issues/detail?id=268
    // need to pass callerFQCN so the log is not generated as if it were always generated from this wrapper class
    log(Log4JLogger.class.getName(), level, "[" + typeInfo + getUserInfo() + " -> " + appInfo + "] " + clean, throwable);
}

See lines 398-453. That's all the escaping that ESAPI provides. I would suggest copying the unit tests as well.

见第 398-453 行。这就是 ESAPI 提供的所有转义。我建议也复制单元测试。

[DISCLAIMER]: I am project co-lead on ESAPI.

[免责声明]：我是 ESAPI 的项目联合负责人。

[1]: https://www.owasp.org/index.php/OWASP_Java_Encoder_Projectand make sure your inputs are properly encoded when going into logging statements--every bit as much as when you're sending input back to the user.

[1]：https: //www.owasp.org/index.php/OWASP_Java_Encoder_Project并确保您的输入在进入日志记录语句时被正确编码——每一点都与您将输入发送回用户时一样。

回答by Vishwas Upadhyay

I am new to Veracode and was facing CWE-117. I understood this error is raised by Veracode when your logger statement has the potential to get attacked via malicious request's parameter values passed in. So we need to removed /r and /n (CRLF) from variables that are getting used in the logger statement.

我是 Veracode 的新手，面对的是 CWE-117。我知道当您的 logger 语句有可能通过传入的恶意请求的参数值受到攻击时，Veracode 会引发此错误。因此，我们需要从 logger 语句中使用的变量中删除 /r 和 /n (CRLF)。

Most of the newbie will wonder what method should be used to remove CRLF from variable passed in logger statement. Also sometime replaceAll() will not work as it is not an approved method by Veracode. Therefore, here is the link to approved methods by Veracode to handles CWE problems. https://help.veracode.com/reader/4EKhlLSMHm5jC8P8j3XccQ/IiF_rOE79ANbwnZwreSPGA

大多数新手会想知道应该使用什么方法从记录器语句中传递的变量中删除 CRLF。此外，有时 replaceAll() 将不起作用，因为它不是 Veracode 批准的方法。因此，这里是 Veracode 批准的处理 CWE 问题的方法的链接。https://help.veracode.com/reader/4EKhlLSMHm5jC8P8j3XccQ/IiF_rOE79ANbwnZwreSPGA

In my case I have used org.springframework.web.util.HtmlUtils.htmlEscape mentioned in the above link and it resolved the problem.

就我而言，我使用了上面链接中提到的 org.springframework.web.util.HtmlUtils.htmlEscape 并解决了问题。

private static final Logger LOG = LoggerFactory.getLogger(MemberController.class);
//problematic logger statement 
LOG.info("brand {}, country {}",brand,country);
//Correct logger statement
LOG.info("brand {}, country {}",org.springframework.web.util.HtmlUtils.htmlEscape(brand),org.springframework.web.util.HtmlUtils.htmlEscape(country));

回答by div11

In order to avoid Veracode CWE 117 vulnerability I have used a custom logger class which uses HtmlUtils.htmlEscape() function to mitigate the vulnerablity. Recommended solution to this problem by Veracode is to use ESAPI loggers but if you dont want to add an extra dependency to your project this should work fine. https://github.com/divyashree11/VeracodeFixesJava/blob/master/spring-annotation-logs-demo/src/main/java/com/spring/demo/util/CustomLogger.java

为了避免 Veracode CWE 117 漏洞，我使用了一个自定义记录器类，它使用 HtmlUtils.htmlEscape() 函数来减轻漏洞。Veracode 对此问题的推荐解决方案是使用 ESAPI 记录器，但如果您不想向项目添加额外的依赖项，这应该可以正常工作。 https://github.com/divyashree11/VeracodeFixesJava/blob/master/spring-annotation-logs-demo/src/main/java/com/spring/demo/util/CustomLogger.java

回答by mzzzzb

If you are using Logback use the replacefunction in your logback config pattern

如果您使用的是 Logback ，请在您的 logback 配置模式中使用替换功能

original pattern

原始图案

<pattern>%d %level %logger : %msg%n</pattern>

with replace

用替换

<pattern>%d %level %logger : %replace(%msg){'[\r\n]', '_'} %n</pattern>

if you want to strip <script>tag as well

如果你也想剥离<script>标签

<pattern>%d %-5level %logger : %replace(%msg){'[\r\n]|&lt;script', '_'} %n</pattern>

This way you dont need to to modify individual log statements.

这样您就不需要修改单个日志语句。

回答by Arpit Pandey

Though I am a bit late but I think it would help those who do not want to use ESAPI library and facing issue only for exception handler class

虽然我有点晚了，但我认为它会帮助那些不想使用 ESAPI 库并且只面临异常处理程序类问题的人

Use apache commons library

使用 apache 公共库

import org.apache.commons.lang3.exception.ExceptionUtils;
LOG.error(ExceptionUtils.getStackTrace(ex));