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

Question

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

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

Veracode scan says that this logging has Improper Output Neutralization for Logs and 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?

P.S. Current logger is 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.

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.Logger=org.owasp.esapi.reference.Log4JLogFactory

Answer 1

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.

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

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

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));

Answer 2

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

Use apache commons library

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

Answer 3

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

Answer 4

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

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

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

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

Answer 5

I have tried with HtmlEscape of org.springframework.web.util.HtmlUtils, but it did not resolve by veracode's vulnerability. Give a try to below solution.

For Java use:

StringEscapeUtils.escapeJava(str)

For Html/JSP use:

StringEscapeUtils.escapeHtml(str)

Please use below package:

import org.appache.commons.lang.StringEscapeUtils;