Is Output Neutralization required when logging C# exception messages to log files?

Question

CWE-117 is the common weakness enumeration for improper output neutralization in logs. My company uses VeraCode to scan for security weaknesses. Veracode indicated that this code had a output neutralization weakness:

catch (Exception e)
{
    _logger.ErrorFormat(_loggerFormat, "An error occurred (while doing something redacted)", e.Message.ToString());
    result = SomeEnum.Exception
}

_logger is an ILogger and uses log4net.

I would argue that an Exception message is NOT untrusted data and therefore I don't have to neutralize it to remove carriage returns and line feeds. I'm not display a string of user input, I'm showing a message from a caught exception.

What am I missing?

Answer 1

Many things in the security field are relative, means they depend on context.

In your case the exception may contain some sensitive data. Only you can decide if this is a weakness or not. A few examples:

• If the exception text contains person names, addresses, account numbers, this may be a security issue in some cases. Normally we don't want to have such data in the logs.
• If the exception text contains generic statement like "User A has no permission for operation B", this is usually a safe text.
• If the exception text is technical like "NullPointerException", it is safe.

Also the exception text can contain stack trace. In some cases disclosing it may be a weakness. For instance, if it contains information about classes and line numbers, it can be possible to find out what version of what library is used. If there is a known security issue in this library version, this can be used for an attack. Again, even if there is such a bug, it can be that exploit requires very specific preconditions and may be you are safe in your case.

Consider such findings not as a problem, but as a hint that there may be a problem. Analyze it, estimate the risks and decide, if the risks are acceptable.

Answer 2

What am I missing?

Maybe just the fact that static analysis tools can give a lot of false positives.

At first glance, your reasoning sounds good to me--unless there is some way for the user to inject data into the generic Exception.Message.