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

Ralph Goers commented on LOG4J2-3037:
-------------------------------------

As I recall we had discussed adding params in LogBuilder but decided against it 
since it just seemed odd to add them separate from the message. This use case 
provides a pretty good reason to add them along with the message methods.  I 
like the concept but am concerned about the drawbacks.

Positives
 # Allows for what you are doing here.
 # Would allow for some args to be provided by a Supplier and others directly, 
which is not possible in the "normal" api.

Negatives
 # Could be handled by wrapping the affected argument with an escape(arg) 
method.
 # Could be lead to some confusion. Some one might want to do

{code:java}
withMessage(new ParameterizedMessage("User provided invalid argument {} to 
service {}")
{code}
or

 
{code:java}
withMessage(() -> new ParameterizedMessage("User provided invalid argument {} 
to service {}"){code}
and the use withArg() to add the arguments. We currently can't support that as 
most messages require the parameters be added in the constructor. You could use 
withMessage to add the Message with its args though, but then the args wouldn't 
be escaped unless the user did it with the mentioned escape(arg) method.

 

 

 

> Add methods for logging untrusted arguments
> -------------------------------------------
>
>                 Key: LOG4J2-3037
>                 URL: https://issues.apache.org/jira/browse/LOG4J2-3037
>             Project: Log4j 2
>          Issue Type: Improvement
>          Components: API
>            Reporter: Marcono1234
>            Priority: Major
>
> While LOG4J2-2511 would prevent log injection, it appears Log4j 2 does not 
> provide any safe means for logging untrusted (potentially malicious) data 
> without it messing with the output. Simply not logging such data is often not 
> an option, because it is important to understand which steps an adversary 
> took.
> It would therefore be good if Log4j 2 provided logging methods for untrusted 
> data, which escape any potentially malicious input.
> Adding such methods to {{Logger}} is probably not an option because it would 
> further bloat the API, making it less usable. However, maybe such methods 
> could be added to {{LogBuilder}}.
> The following API would allow this (and could unrelated to this request also 
> support logging primitive arguments without boxing):
> {code}
> interface LogBuilder {
>     interface MessageWithArgsBuilder {
>         MessageWithArgsBuilder withArg(Object arg);
>         MessageWithArgsBuilder withUntrustedArg(Object arg);
>         void log();
>     }
>     MessageWithArgsBuilder withMessage(String messageTemplate);
> }
> {code}
> The usage would then look like this:
> {code}
> logger.atWarn()
>   .withMessage("User provided invalid argument {} to service {}")
>   .withUntrustedArg(arg)
>   .withArg(service)
>   .log();
> {code}
> The implementation of {{withUntrustedArg}} would then only allow a subset of 
> the ASCII chars, encasing the argument with double quotes {{"}}, and 
> replacing any non allowed characters with their Unicode escape sequence 
> {{\uXXXX}}, adding to any existing escape sequences an additional {{u}}, e.g.
> {noformat}
> test" \u1234
> {noformat}
> would become
> {noformat}"test\u0022 \uu1234"
> {noformat}
> in the output.



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

Reply via email to