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

Reply via email to