This is an automated email from the ASF dual-hosted git repository.

coheigea pushed a commit to branch 4.1.x-fixes
in repository https://gitbox.apache.org/repos/asf/cxf.git


The following commit(s) were added to refs/heads/4.1.x-fixes by this push:
     new 610b7d5a828 More JNDI validation (#3150)
610b7d5a828 is described below

commit 610b7d5a828725860590e4d3c8eff18db098cf65
Author: Colm O hEigeartaigh <[email protected]>
AuthorDate: Wed May 27 15:02:11 2026 +0100

    More JNDI validation (#3150)
    
    (cherry picked from commit b6941a38027408de8ea6ce95859c5ac8bdd86102)
---
 .../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
+    }
+}

Reply via email to