0

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?

2
  • 1
    What is the format _loggerFormat?
    – hft
    Jun 10, 2021 at 19:40
  • 1
    @hft It turns out that was the problem. _loggerFormat contained possible CRLF issues. Jul 30, 2021 at 12:47

2 Answers 2

2

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.

1

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.

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .