This is an automated email from the ASF dual-hosted git repository.
coheigea pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/cxf.git
The following commit(s) were added to refs/heads/main by this push:
new b6941a38027 More JNDI validation (#3150)
b6941a38027 is described below
commit b6941a38027408de8ea6ce95859c5ac8bdd86102
Author: Colm O hEigeartaigh <[email protected]>
AuthorDate: Wed May 27 15:02:11 2026 +0100
More JNDI validation (#3150)
---
.../apache/cxf/transport/jms/JMSConfigFactory.java | 1 +
.../apache/cxf/transport/jms/util/JndiHelper.java | 45 +++++++
.../cxf/transport/jms/JMSConfigFactoryTest.java | 23 +++-
.../cxf/transport/jms/util/JndiHelperTest.java | 138 +++++++++++++++++++++
4 files changed, 205 insertions(+), 2 deletions(-)
diff --git
a/rt/transports/jms/src/main/java/org/apache/cxf/transport/jms/JMSConfigFactory.java
b/rt/transports/jms/src/main/java/org/apache/cxf/transport/jms/JMSConfigFactory.java
index c3c9266513f..5a95b40d406 100644
---
a/rt/transports/jms/src/main/java/org/apache/cxf/transport/jms/JMSConfigFactory.java
+++
b/rt/transports/jms/src/main/java/org/apache/cxf/transport/jms/JMSConfigFactory.java
@@ -190,6 +190,7 @@ public final class JMSConfigFactory {
if (transactionManagerJndiName == null) {
return null;
}
+ JndiHelper.validateJndiName(transactionManagerJndiName);
try {
InitialContext ictx = new InitialContext();
return (TransactionManager)ictx.lookup(transactionManagerJndiName);
diff --git
a/rt/transports/jms/src/main/java/org/apache/cxf/transport/jms/util/JndiHelper.java
b/rt/transports/jms/src/main/java/org/apache/cxf/transport/jms/util/JndiHelper.java
index 0f86ead1474..63721cae82a 100644
---
a/rt/transports/jms/src/main/java/org/apache/cxf/transport/jms/util/JndiHelper.java
+++
b/rt/transports/jms/src/main/java/org/apache/cxf/transport/jms/util/JndiHelper.java
@@ -23,6 +23,7 @@ import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Properties;
+import java.util.Set;
import java.util.function.Predicate;
import javax.naming.Context;
@@ -44,6 +45,32 @@ public class JndiHelper {
*/
private static final String DEFAULT_JMS_PROTOCOLS =
"vm,tcp,nio,ssl,http,https,ws,wss";
private static final List<String> ALLOWED_PROTOCOLS;
+
+ /**
+ * JNDI environment properties that must not be supplied from untrusted
input.
+ * <p>
+ * Note: {@code java.naming.factory.initial} (INITIAL_CONTEXT_FACTORY) is
intentionally
+ * absent. It is a first-class, documented configuration parameter — every
JNDI-based JMS
+ * deployment sets it to the broker's context factory (e.g. ActiveMQ,
Artemis, WebLogic).
+ * It is safe to allow because any attempt to redirect lookups to a remote
host via a
+ * substitute factory still requires setting PROVIDER_URL to a non-JMS
scheme, which the
+ * protocol allowlist check below independently rejects.
+ * </p>
+ * <ul>
+ * <li>{@code java.naming.factory.object} — object factories reconstruct
objects returned
+ * by a lookup and can deserialize remote payloads.</li>
+ * <li>{@code java.naming.factory.state} — state factories run during
bind/rebind and
+ * can trigger arbitrary serialization logic.</li>
+ * <li>{@code java.naming.factory.url.pkgs} — injects packages for URL
context factory
+ * resolution and could register a handler for an otherwise-allowed
scheme.</li>
+ * </ul>
+ */
+ private static final Set<String> BLOCKED_ENV_KEYS = Set.of(
+ "java.naming.factory.object",
+ "java.naming.factory.state",
+ "java.naming.factory.url.pkgs"
+ );
+
private Properties environment;
static {
@@ -68,6 +95,13 @@ public class JndiHelper {
public JndiHelper(Properties environment) {
this.environment = environment;
+ // Reject properties that could be used to redirect or hijack the JNDI
lookup
+ for (String blocked : BLOCKED_ENV_KEYS) {
+ if (environment.containsKey(blocked)) {
+ throw new IllegalArgumentException("Disallowed JNDI
environment property: " + blocked);
+ }
+ }
+
// Avoid unsafe protocols if they are somehow misconfigured
String providerUrl = environment.getProperty(Context.PROVIDER_URL);
if (providerUrl != null && !providerUrl.isEmpty()
@@ -76,8 +110,19 @@ public class JndiHelper {
}
}
+ /**
+ * Rejects lookup names that contain a URL scheme (e.g. ldap://, rmi://),
which would
+ * redirect the lookup to a remote server and enable JNDI injection.
+ */
+ public static void validateJndiName(String name) {
+ if (name != null && name.contains("://")) {
+ throw new IllegalArgumentException("JNDI name must not contain a
URL: " + name);
+ }
+ }
+
@SuppressWarnings("unchecked")
public <T> T lookup(final String name, Class<T> requiredType) throws
NamingException {
+ validateJndiName(name);
Context ctx = createInitialContext();
try {
Object located = ctx.lookup(name);
diff --git
a/rt/transports/jms/src/test/java/org/apache/cxf/transport/jms/JMSConfigFactoryTest.java
b/rt/transports/jms/src/test/java/org/apache/cxf/transport/jms/JMSConfigFactoryTest.java
index 8ac1f79d263..0c4af03be86 100644
---
a/rt/transports/jms/src/test/java/org/apache/cxf/transport/jms/JMSConfigFactoryTest.java
+++
b/rt/transports/jms/src/test/java/org/apache/cxf/transport/jms/JMSConfigFactoryTest.java
@@ -45,11 +45,11 @@ public class JMSConfigFactoryTest extends AbstractJMSTester
{
env.put(Context.PROVIDER_URL, "ldap://127.0.0.1:12345");
// Allow following referrals (important for LDAP injection)
env.put(Context.REFERRAL, "follow");
-
+
JMSConfiguration jmsConfig = new JMSConfiguration();
jmsConfig.setJndiEnvironment(env);
jmsConfig.setConnectionFactoryName("objectName");
-
+
try {
jmsConfig.getConnectionFactory();
Assert.fail("JNDI lookup should have failed");
@@ -103,6 +103,25 @@ public class JMSConfigFactoryTest extends
AbstractJMSTester {
// TODO Check JNDI lookup
}
+ @Test
+ public void testTransactionManagerJndiInjectionRejected() {
+ // Ensure URL-style JNDI names (e.g. ldap://, rmi://) are rejected to
prevent
+ // JNDI injection via getTransactionManagerFromJndi.
+ for (String malicious : new String[]{"ldap://attacker.com/exploit",
+ "rmi://attacker.com/exploit",
+ "corba://attacker.com/exploit"}) {
+ Bus testBus = BusFactory.newInstance().createBus();
+ JMSEndpoint endpoint = new
JMSEndpoint("jms:queue:Foo.Bar?jndiTransactionManagerName="
+ + malicious);
+ try {
+ JMSConfigFactory.createFromEndpoint(testBus, endpoint);
+ Assert.fail("Expected IllegalArgumentException for JNDI name:
" + malicious);
+ } catch (IllegalArgumentException e) {
+ Assert.assertTrue(e.getMessage().contains("JNDI name must not
contain a URL"));
+ }
+ }
+ }
+
@Test
public void testConcurrentConsumers() {
JMSEndpoint endpoint = new
JMSEndpoint("jms:queue:Foo.Bar?concurrentConsumers=4");
diff --git
a/rt/transports/jms/src/test/java/org/apache/cxf/transport/jms/util/JndiHelperTest.java
b/rt/transports/jms/src/test/java/org/apache/cxf/transport/jms/util/JndiHelperTest.java
new file mode 100644
index 00000000000..22996d8ac84
--- /dev/null
+++
b/rt/transports/jms/src/test/java/org/apache/cxf/transport/jms/util/JndiHelperTest.java
@@ -0,0 +1,138 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.cxf.transport.jms.util;
+
+import java.util.Properties;
+
+import javax.naming.Context;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+public class JndiHelperTest {
+
+ // --- validateJndiName ---
+
+ @Test
+ public void testValidateJndiNamePlainNamesAllowed() {
+ // Ordinary local JNDI names must not be rejected
+ JndiHelper.validateJndiName("TransactionManager");
+ JndiHelper.validateJndiName("java:comp/env/jms/MyQueue");
+ JndiHelper.validateJndiName("java:/TransactionManager");
+ JndiHelper.validateJndiName(null);
+ }
+
+ @Test
+ public void testValidateJndiNameRemoteUrlRejected() {
+ for (String malicious : new String[]{
+ "ldap://attacker.com/exploit",
+ "ldaps://attacker.com/exploit",
+ "rmi://attacker.com/exploit",
+ "iiop://attacker.com/exploit",
+ "corba://attacker.com/exploit",
+ "dns://attacker.com/exploit"}) {
+ try {
+ JndiHelper.validateJndiName(malicious);
+ Assert.fail("Expected IllegalArgumentException for: " +
malicious);
+ } catch (IllegalArgumentException e) {
+ Assert.assertTrue(e.getMessage().contains("JNDI name must not
contain a URL"));
+ }
+ }
+ }
+
+ // --- constructor: blocked environment keys ---
+
+ @Test
+ public void testInitialContextFactoryAllowed() {
+ // INITIAL_CONTEXT_FACTORY is a first-class config parameter set by
every JNDI-based
+ // JMS deployment (e.g. ActiveMQ, Artemis) and must not be blocked.
+ Properties env = new Properties();
+ env.put(Context.PROVIDER_URL, "vm://localhost");
+ env.put(Context.INITIAL_CONTEXT_FACTORY,
+
"org.apache.activemq.artemis.jndi.ActiveMQInitialContextFactory");
+ new JndiHelper(env); // must not throw
+ }
+
+ @Test
+ public void testBlockedObjectFactoryRejected() {
+ Properties env = new Properties();
+ env.put("java.naming.factory.object",
"com.attacker.EvilObjectFactory");
+ try {
+ new JndiHelper(env);
+ Assert.fail("Expected IllegalArgumentException for
java.naming.factory.object");
+ } catch (IllegalArgumentException e) {
+ Assert.assertTrue(e.getMessage().contains("Disallowed JNDI
environment property"));
+
Assert.assertTrue(e.getMessage().contains("java.naming.factory.object"));
+ }
+ }
+
+ @Test
+ public void testBlockedStateFactoryRejected() {
+ Properties env = new Properties();
+ env.put("java.naming.factory.state", "com.attacker.EvilStateFactory");
+ try {
+ new JndiHelper(env);
+ Assert.fail("Expected IllegalArgumentException for
java.naming.factory.state");
+ } catch (IllegalArgumentException e) {
+ Assert.assertTrue(e.getMessage().contains("Disallowed JNDI
environment property"));
+
Assert.assertTrue(e.getMessage().contains("java.naming.factory.state"));
+ }
+ }
+
+ @Test
+ public void testBlockedUrlPkgsRejected() {
+ Properties env = new Properties();
+ env.put("java.naming.factory.url.pkgs", "com.attacker");
+ try {
+ new JndiHelper(env);
+ Assert.fail("Expected IllegalArgumentException for
java.naming.factory.url.pkgs");
+ } catch (IllegalArgumentException e) {
+ Assert.assertTrue(e.getMessage().contains("Disallowed JNDI
environment property"));
+
Assert.assertTrue(e.getMessage().contains("java.naming.factory.url.pkgs"));
+ }
+ }
+
+ @Test
+ public void testUnsafeProviderUrlRejected() {
+ Properties env = new Properties();
+ env.put(Context.PROVIDER_URL, "ldap://attacker.com:389");
+ try {
+ new JndiHelper(env);
+ Assert.fail("Expected IllegalArgumentException for unsafe
PROVIDER_URL");
+ } catch (IllegalArgumentException e) {
+ Assert.assertTrue(e.getMessage().contains("Unsafe protocol in JNDI
URL"));
+ }
+ }
+
+ @Test
+ public void testSafeProviderUrlAllowed() {
+ // Allowed JMS broker protocols must not be rejected
+ for (String safe : new String[]{"vm://localhost",
"tcp://localhost:61616",
+ "ssl://localhost:61617",
"nio://localhost:61618"}) {
+ Properties env = new Properties();
+ env.put(Context.PROVIDER_URL, safe);
+ new JndiHelper(env); // must not throw
+ }
+ }
+
+ @Test
+ public void testEmptyPropertiesAllowed() {
+ new JndiHelper(new Properties()); // must not throw
+ }
+}