[
https://issues.apache.org/jira/browse/LOG4J2-3210?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17458216#comment-17458216
]
Bernd Eckenfels commented on LOG4J2-3210:
-----------------------------------------
I agree with removing the lookup and disabling jndi and also think hardening
JNDI is very hard.
However I think for the JMSManager case where JNDI is needed some hardening
should be done. One option would be to allow only globally (ion the components)
defined trusted URLs and not make then defineable inside the logger config.
> Not effective deserialisation controls
> --------------------------------------
>
> Key: LOG4J2-3210
> URL: https://issues.apache.org/jira/browse/LOG4J2-3210
> Project: Log4j 2
> Issue Type: Bug
> Components: Core
> Affects Versions: 2.15.0
> Reporter: Bernd Eckenfels
> Priority: Major
> Labels: SECURITY, jndi
>
> The new JNDILookup protection by disabling the lookups and by disabling JNDI
> is effective. However, since you still keep the JNDI Lookup Code around I had
> a look at the new restrictions, and I don’t think they are affective:
> a) there is no protection (allowed host/port) for the RMI protocol. It should
> probably use the same allowed hosts than LDAP does
> b) when checking the allowed class in the LDAP case you asume the class
> Attribute is actually the class which is serialized. This must not be true,
> an attacker can pretend their object is a java.lang.Stringˋ which will pass
> your filter. Later on when you use lookup the JNDI code will try to
> deserialize the object - and then it is too late.
> My suggesting would be to not use context.lookup() if not needed and/or
> disallow all forms of serialized data (which might not work for the
> JMSManager case?). Anyway, for the `${jndi:`-lookup case I can imagine only
> string attributes are needed anyway?
> Is it safe to allow the whole java: context and how would you restrict
> absolute /… names without an prefix? (At the moment I guess they will get the
> java:env prefix since there is no logic to skip that with leading slash?)
> Sidenote: there is also a osgi: scheme which could be useful, but also
> dangerous.
> BTW the disabled JNDIManager with the context==null, it should have a comment
> what it’s meaning is „// create unusable JNDIManager with context == null“
> and I am also not sure if it is only created with this code path.. maybe
> better to check the the isJndiEnabled also in the constructor? (I sent a pull
> request with a minor rename of the isIs.. method as well).
--
This message was sent by Atlassian Jira
(v8.20.1#820001)
