[jira] [Commented] (LOG4J2-3037) Add methods for logging untrusted arguments

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

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