Since Piotrs response went to spam (thanks for confirming) I'd like to make
sure you reveived Volkans questions as well. Please let me know if you did.
If you didn't, he sent his response to the mailing list, if you need help
subscribing, please let me know
On Mon, Oct 9, 2023, at 22:28, Volkan Yazıcı wrote:
> *[I am sharing my earlier response (almost) verbatim below.]*
>
> I would like to address your both old and the most recent email *myself* –
> that is, it only reflects my personal view, and not of the PMC.
>
>> A HTML-safe layout is only achieved if
>
>> defined akin to:
>
>>
>
>> <PatternLayout pattern="%d{HH:...
>
>
> The definition of *HTML-safe* needs some explanation here. If you mean, the
> rendering should be a valid HTML document where the input is sufficiently
> escaped, then certainly the output of the `PatternLayout` configuration you
> shared won't produce that. Indeed the implicit injection of the stack trace
> is unexpected, yet you already garbled the HTML-safe content with the first
> directives you provided. Imagine my thread name is `<html>`, etc. My point
> is, if you want your layout to produce a valid HTML for each rendered log
> event, you should be using `HtmlLayout`. The same applies to JSON too. You
> should use `JsonTemplateLayout`, not `<PatternLayout
> pattern="%enc{%m}{JSON}%n"/>`.
>
>
>> Would Log4j be willing to improve the usability of encoding in pattern
>> layouts to make it less likely for users to shoot themselves in the foot?
>
>
> We provide best in the class support for JSON, HTML, etc. with their
> associated dedicated layouts. If users insist on using `PatternLayout` for
> those purposes, it feels to me somebody is stubbornly trying to pass SQL
> arguments with string concatenation.
>
>
> Nevertheless, if you have any proposals on _"improving the usability of
> encoding in pattern layouts to make it less likely for users to shoot
> themselves in the foot"_, you are more than welcome! The entire Log4j crew
> would be happy to assist you for such contributions.
>
>
>> I did go ahead and create a proof-of-concept encoder for
>
>> log4j that securely encodes exceptions without completely
>
>> mangling the stack traces:
>
>>
>
>> https://github.com/vlkl-sap/log4j-encoder
>
>>
>
>> There are two different implementations of the encoder with
>
>> different trade-offs (to be discussed). I also implemented a
>
>> new, more encompassing text encoder, based on URL
>
>> encoding, but this aspect is independent.
>
>
> Before writing any code, would you mind helping us with the following
> questions, please?
>
>
> 1. Do you have a use case? If so, where does `HtmlLayout` fall short of
> addressing it?
> 2. Assuming `HtmlLayout` doesn't address your needs, can we [in a
> backward-compatible manner] improve `HtmlLayout` to make it work for you?
> 3. Can we [in a backward-compatible manner] incorporate your
> `PatternLayout` changes?
>
> Kind regards.
>
>
> On Mon, Oct 9, 2023 at 5:24 PM Klebanov, Vladimir
> <vladimir.kleba...@sap.com.invalid> wrote:
>
>> Thanks, Piotr. I don't know what happened to your replies (maybe the spam
>> filter dropped them), but I am happy that we recovered from that now.
>>
>> Log injections are definitely security issues, but if you prefer to talk
>> about them in the open, I will follow suit.
>>
>> For context: a log injection occurs when an application logs user-supplied
>> data (which is often the case). Attacker can exploit log injection to forge
>> log records and impede forensics or exploit potential vulnerabilities in
>> log-processing systems. There is a variety of string classes that attackers
>> can try to inject, including newlines, ANSI sequences, Unicode direction
>> markers, Unicode homographs, JavaScript, PHP, etc.
>>
>> Ideally, applications defend against log injection attacks by encoding
>> (aka escaping) user-supplied data before logging. The specific encoding
>> depends on the desired level of protection. URL-encoding, for instance,
>> would protect against all of the above-mentioned attack classes, but weaker
>> encodings may be sometimes acceptable as well.
>>
>> A natural place to implement encoding is in the pattern layout
>> configuration. Some encoding pattern converters are already available in
>> log4j, but there are still gaps that I would like to help fill. I think
>> there are roughly three of them:
>>
>> 1. The documentation should more prominently explain the issue. Today,
>> most users would probably think that the following layout is HTML-safe,
>> while it's not:
>> <PatternLayout pattern="%d{HH:mm:ss.SSS} [%t] %-5level -
>> %enc{%m}{HTML}%n"/>
>>
>> 2. The HTML encoder is not always sufficient. I would like to see an
>> addition of a stricter one, such as a URL-encoder.
>>
>> 3. The current encoders encode all structured data (like the complete
>> exception stacktrace) and not just the injection-prone parts (i.e., the
>> exception message). This means I cannot replace the insecure layout above
>> with the secure layout
>>
>> <PatternLayout pattern="%d{HH:mm:ss.SSS} [%t] %-5level -
>> %enc{%m}{HTML} %enc{%xEx}{HTML}%n"/>
>>
>> without changing how logs are parsed (as the stack frames will not be
>> separated by newlines anymore).
>>
>> I have created a PoC implementation of an improved encoder, but I would
>> obviously need help to make it productive. Is anyone here interested in
>> that? Questions and comments are welcome as well.
>>
>> Thanks,
>> Vladimir
>>
>>
>> -----Original Message-----
>> From: Piotr P. Karwasz <piotr.karw...@gmail.com>
>> Sent: Thursday, 5 October 2023 22:06
>> To: dev@logging.apache.org; Klebanov, Vladimir <vladimir.kleba...@sap.com>
>> Subject: Re: [log4j] Improving log4j security
>>
>> [You don't often get email from piotr.karw...@gmail.com. Learn why this
>> is important at https://aka.ms/LearnAboutSenderIdentification ]
>>
>> Hi Vladimir,
>>
>> On Thu, 5 Oct 2023 at 21:47, Klebanov, Vladimir
>> <vladimir.kleba...@sap.com.invalid> wrote:
>> > I would like to contribute some code in order to make log4j usage more
>> secure. I have now sent two emails to the log4j security team but did not
>> receive a response. Is anybody here interested? How can we discuss this
>> further?
>>
>> Both times (10 Aug 2023, 23:19 and 29 Aug 2023, 20:49) we sent an
>> answer to your address at sap.com.
>>
>> Anyway the general consensus was that the issue with generating HTML
>> using PatternLayout does not constitute a security problem and you can
>> discuss it on this mailing list or file an issue in Github issues.
>>
>> Piotr
>>
