Earlier quoted context omitted.
Here's the alleged original Jira ticket: https://issues.apache.org/jira/browse/LOG4J2-313 PS: Don't post snarky comments on an 8-year old Jira ticket, please.
I have read that ticket and the JNDI documentation multiple times and I still can't picture what it would look like to choose to use this. Can anyone show me a codebase that utilises this feature so we can see why?
Some background:
JNDI alone is fine. You want to load code remotely and run it? Fine. You can do that already in other ways. No issue with JNDI here, go ahead and use it on your server in a controlled way that doesn't involve user input.
Log4j has a 'routing appender' that can conditionally write to different logs depending on the content being logged. https://logging.apache.org/log4j/2.x/manual/appenders.html#R...
I can see a use to string match and send logs different ways.
Now unfortunately this patch flat out uses the 'routing appender' to look for incoming log statements with the pattern ${jndi:logging/context-name} and load that remote JNDI class.
This is such a terrible idea that doesn't pass the sniff test. The person who approved this should have simply read the description of what it does. After matching the pattern ${jndi:logging/context-name} it puts that match into a string 'key' and runs ctx.lookup(convertJndiName(key));
It's similar to someone submitting a patch that says "I want to run eval(user_input)". The only difference is that lookup(convertJndiName()) is a little bit obfuscated since it's not called eval(). I guess the review could be mistaken that it's harmless?. Still i think it's a bad smell. I'm worried for this project. It's probably worth going through everything that 'implements StrLookup' and seeing what they do in the 'lookup(final LogEvent event, final String key)' function. Both event and key are user generated content.