0

I am trying to get the exception details by logging it using lombok extern Slf4j. But found an issue in coverity scan as below.

This is a security audit finding. CID 227846 (#1 of 1): Log injection (LOG_INJECTION). A tainted string ex is stored in logs. This may allow for an attacker to forge log messages to confuse automated log parsing tools or humans trying to diagnose an attack or other problem. The value is used unsafely in bytecode, which cannot be displayed. Log injection vulnerabilities can be addressed by validating that the user-controllable input conforms to expectations.

log.error(Constants.EXCEPTION_OCCURRED_MSG, ex);

I rarely found options to resolve this issue. Does ESAPI or Apache log4j Audit fit here. Please suggest.

Sripada A
  • 3
  • 2

2 Answers2

0

I can't speak much for Apache Log4J Audit because I have never looked at it (although, a 20 second perusal of its main web page seems to indicate it is more an attempt at structured logging messages that SIEMs would then know how to parse, rather than any sort of "safe" logging that does filtering / encoding / etc.). As for ESAPI, while ESAPI does handle "safe logging" to some degree, IIRC (didn't verify) it really doesn't do much with exceptions. Mainly ESAPI's logging trusts the exception stack and more focuses on the error message itself. Generally for secure designs, user data should never be placed in exception messages unless it is verified. But that sort of verification is not something that a general logging framework can do. To the logging framework, it has to be able to handle any Exception (or perhaps Throwable, YMMV) and any String and by the time it gets to a logging framework, the specific context is of how something should be validated is lost.

ESAPI's "safe logging" mainly consists of replacing "\r" or "\n" characters as part of the logged 'message' string (not Exception) with "_" (to prevent log injection) and optionally to do HTML entity output encoding on the message (in case one intends to peruse the log messages in a browser; the intent being to prevent XSS attacks via log messages). Although you could, with enough effort, extend it to do other things (e.g., filter out ESC sequences, etc.).

Ultimately, to prevent log injection attacks completely though, you must verify the all untrusted data before logging it. ESAPI's Validator can assist you with that, but you as a developer still need to call it with the proper validation criteria at the right moment.

Added 12/29/201: As far as using ESAPI's Validator, I did not mean to do output encoding using ESAPI's Encoder. Instead, you would create a regex to white-list your expected data and put that into your validation.properties and then call one of the Validator.getValidInput() methods. If you don't get a ValidationException thrown, the output should be "safe" according to your regex. (Note that you may need to do that in several places with many different regular expressions.) OTOH, I can't promise you that will make your Coverity scan happy as it can't possibly know whether or not the regex that you provide is an appropriate one or not. (Nor do I think it will do that deep of a data flow analysis to consider it 'safe' for use.)

Kevin W. Wall
  • 1,347
  • 7
  • 7
  • I can't do much more than dogpile on Kevin's answer here. If Coverity is flagging on that call, then it must be finding a way that user input is making its way into the stack trace, and from my distant vantage point, I find it really hard to envisage how user data could be used in this way other than to have the user specify classes to be loaded by a classloader, which is an even worse problem. So investigate how a user can manipulate that exception and you have your answer. Don't be the guy who just makes library calls to appease the SAST tool! – avgvstvs Dec 23 '21 at 17:39
  • I tried using ESAPI but no luck. May be I am missing something. I included ESAPI.properties and validation.properties file in classpath. Used ESAPI encoder to the exception as ESAPI.encoder().encodeForHTML(ex.toSring()). Is there anything that I missed in validation.properties file? – Sripada A Dec 28 '21 at 12:56
  • There is a property in ESAPI.properties that pretty much gives you HTML output encoding for free. Just set Logger.LogEncodingRequired=true. It is false by default. – Kevin W. Wall Dec 30 '21 at 01:24
  • @KevinW.Wall Thanks for your quick and detailed response. The property which I am logging has no validation restrictions. That's where I need to encode. Able to resolve most of the issues and left with one. Can you share any doc related to ESAPI which has full details. – Sripada A Dec 30 '21 at 17:14
  • @KevinW.Wall I see ESAPI Library has High-Risk License identified in Veracode. Please comment on this. Will this create any issues later? – Sripada A Dec 30 '21 at 17:15
  • @SripadaA - I'm sorry, but I don't have access to Veracode. So what is a "High-Risk License"? ESAPI is released under the New BSD license (i.e., https://opensource.org/licenses/BSD-3-Clause), which is one of the most liberal FOSS licenses there is. – Kevin W. Wall Dec 31 '21 at 22:23
  • @SripadaA - I'm not sure what exactly you are looking for. Perhaps you can share a fuller code snippet that creates the warning, if not here, than you can email me. (My email address is pretty easy to find if you just search for my name along with esapi and email.) However, if you set Logger.LogEncodingRequired=true, the same HTML entity output encoding you were doing will be done by ESAPI's loggers. Veracode may not understand that and thus might not remove its taint flag & therefore may continue to flag the issue. (I.e., it may be a false positive.) – Kevin W. Wall Dec 31 '21 at 22:30
  • @KevinW.Wall. Thanks for suggesting ESAPI. Able to resolve most of the Log injection issues except one. As you suggested I updated Logger.LogEncodingRequired to true. For resolving other issues I have to sanitize the parameters in that exception. For example, if any mediatype is not supported, then controlleradvice checks for HttpMediaTypeNotSupportedException, where I sanitized log.error(Constants.EXCEPTION_OCCURRED_MSG, ApiUtils.sanitize(ex.getContentType()), ex). Similarly I tried for ConstraintViolationException but failed to resolve. – Sripada A Jan 11 '22 at 08:50
  • @Kevin.Need help for below `@ExceptionHandler(ConstraintViolationException.class) public ResponseEntity handleConstraintVoilationException(ConstraintViolationException ex){ log.error("Ex", ex); //LogInjection declare Set errFlds, patternErrFlds; ex.getConstraintViolations().forEach(cv -> { log.error("Ex", ex.getMessage()); //Log injection String name=StringUtils.unqualify(cv.getPropertyPath().toString()) if (ex.getMessage().contains(Constants.PATTERN)) patternErrFlds.add(name); else errFlds.add(name); }); return buildResponseEntity(errFlds,patternErrFlds) }` – Sripada A Jan 11 '22 at 11:01
0

I think I would need more details on what exactly that error is looking for. For instance, if the concern is that the exception message associated with 'ex' contains PII or other sensitive information that may be logged, that's a lot tougher unless you have control of the exception that gets thrown. OTOH, if the control is inserting an attacker could insert something like the Log4shell exploit that attacked the insecure deserialization that (apparently) no one had realized was going on since 2013, that's not something that can be defended easily, if at all because the problem is in the underlying logging system for stuff like that. Generally the best way to approach this is to talk to talk to the security auditors and ask them specifically what they would consider as remediation for this. That is what sort of sanitization or validation do they expect? Also, you should understand that with management approval, you generally can just record the risk somewhere and then decide to "accept it". Help your management to understand most "Log Injection" vulnerabilities are not anywhere as severe as what we've seen with the recent string of Log4J 2 vulnerabilities. It's just that the Log4Shell exploit has everyone on edge for the past month or so. If you decide to accept the risk, you may wish to track it in something like a "risk register" that is periodically reviewed, but you also will want to mark it in the auditing tool as an "accepted risk".

Without knowing a lot more specific details myself, I'm afraid I cannot give you a more specific answer. I'm willing to try to provide more help in a different forum where I can involve my ESAPI colleagues. In particular Jeremiah Stacey, who did a rewrite of the ESAPI Logger may have some ideas, but I don't think he monitors SO, so email would probably be a better forum to start with.

Hope that helps a bit. -kevin

Kevin W. Wall
  • 1,347
  • 7
  • 7