[jira] [Commented] (LOG4J2-2511) Turn Log Injection Defenses On By Default

["Marcono1234 (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/LOG4J2-2511?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17296998#comment-17296998 ] 

Marcono1234 commented on LOG4J2-2511:
-------------------------------------

I can only agree with the reporter, log injection and log forging prevention should ideally be done by the logging framework. Asking the user to do this
- is error-prone; e.g. merely replacing {{\n}} would miss other control characters
- is easy to forget
- clutters user code

The implementation of this would probably have to depend on the used Appender (some appenders have by default a vulnerable pattern) and the Layout (regardless of used Appender). Limiting it to Pattern Layout only would probably leave other layouts or appenders vulnerable.

For Pattern Layout, Console Appender and all of the file Appenders, it would be good if Log4j 2 by default replaced all [control characters|https://docs.oracle.com/en/java/javase/15/docs/api/java.base/java/lang/Character.html#isISOControl(char)]. Either the control characters should be removed, or possibly replaced with some kind of unambiguous escape sequence; maybe using the {{\uXXXX}} escape sequences and adding an additional {{u}} to any existing escape sequences, e.g. {{\u1234}} to {{\uu1234}} (similar to how the Java command line tools do / did it to create ASCII only source code).
Maybe the only exception could be {{\n}} and {{\r}}, which could be handled in a different way. For example for these characters there could be an 'indent' mode which inserts an indentation (e.g. 4 spaces) behind line terminators ({{\n}}, {{\r\n}}, {{\r}}) so that the line terminator is preserved (which might be desired for trusted log messages), but consecutive lines are indented to prevent log injection.

So maybe (at least for the layout and appenders listed above), the following {{LogInjectionPrevention}} (new enum) values would make sense?
- {{NONE}}
- {{ESCAPE_AND_INDENT}} (default)
- {{ESCAPE_ALL}}

Note that this probably also has to apply to
- {{Throwable}} messages logged by Log4j 2
- Lookups whose data might be tainted (list might be incomplete):
  - NDC, MDC
  - MapLookup
  - Structured Data Lookup

> Turn Log Injection Defenses On By Default
> -----------------------------------------
>
>                 Key: LOG4J2-2511
>                 URL: https://issues.apache.org/jira/browse/LOG4J2-2511
>             Project: Log4j 2
>          Issue Type: Improvement
>          Components: Pattern Converters
>    Affects Versions: 2.11.1
>            Reporter: Dave Wichers
>            Priority: Minor
>              Labels: Security
>
> Per: [https://logging.apache.org/log4j/log4j-2.8/manual/layouts.html] - there is a new encoding scheme introduced in 2.10.0 (by https://issues.apache.org/jira/browse/LOG4J2-1203) that allows users to encode plain logging output with *enc*{_pattern_}\{CRLF} to avoid Log Injection attacks ([https://www.owasp.org/index.php/Log_Injection)|https://www.owasp.org/index.php/Log_Injection).]. While it is great to have this available, most developers won't be aware of the risk of Log Injection so won't do anything about it.
> I recommend that Log4J2 enable this encoding by default if no other encoding scheme is specified. It shouldn't hurt plain text logging by defending against this attack automatically. However, to allow people to disable it in case they really don't want this I suggest creating an encoding scheme like \{NONE} that explicitly disables this new default behavior which people can use to turn it off.
>  



--
This message was sent by Atlassian Jira
(v8.3.4#803005)