This is an automated email from the ASF dual-hosted git repository.
cshannon pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/activemq.git
The following commit(s) were added to refs/heads/main by this push:
new bedb10801c Fix FactoryFinder path resolution in Windows (#1831)
bedb10801c is described below
commit bedb10801c4d11e12f61ef8a9d9cf3407be50e97
Author: Christopher L. Shannon <[email protected]>
AuthorDate: Thu Mar 26 09:55:09 2026 -0400
Fix FactoryFinder path resolution in Windows (#1831)
This update ensures forward slashes are alow used for classpath
resolution, even on all Windows versions. Also add extra validation
to prevent path separators (slashes) in factory key names. Lastly added
some more tests for validation
This closes #1830
---
.../org/apache/activemq/util/FactoryFinder.java | 15 ++++++-
.../apache/activemq/util/FactoryFinderTest.java | 50 +++++++++++++++++++++-
.../transport/stomp/ProtocolConverterTest.java | 9 +++-
.../activemq/web/QueueBrowseServletTest.java | 16 +++++++
4 files changed, 85 insertions(+), 5 deletions(-)
diff --git
a/activemq-client/src/main/java/org/apache/activemq/util/FactoryFinder.java
b/activemq-client/src/main/java/org/apache/activemq/util/FactoryFinder.java
index 594af7e257..75ef517750 100644
--- a/activemq-client/src/main/java/org/apache/activemq/util/FactoryFinder.java
+++ b/activemq-client/src/main/java/org/apache/activemq/util/FactoryFinder.java
@@ -219,7 +219,12 @@ public class FactoryFinder<T> {
return requiredType;
}
- private String resolvePath(final String key) throws InstantiationException
{
+ String resolvePath(final String key) throws InstantiationException {
+ // Validate the key has no path separators
+ if (containsPathSeparators(key)) {
+ throw new InstantiationException("Provided key may not contain
path separators");
+ }
+
// Normalize the base path with the given key. This
// will resolve/remove any relative ".." sections of the path.
// Example: "/dir1/dir2/dir3/../file" becomes "/dir1/dir2/file"
@@ -231,7 +236,13 @@ public class FactoryFinder<T> {
throw new InstantiationException("Provided key escapes the
FactoryFinder configured directory");
}
- return resolvedPath.toString();
+ // replace any backslashes with forward slashes (may happen in some
Windows versions)
+ return resolvedPath.toString().replace("\\", "/");
+ }
+
+ public static boolean containsPathSeparators(String str) {
+ // Check for forward and backslash
+ return str.contains("/") || str.contains("\\");
}
public static String buildAllowedImpls(Class<?>...classes) {
diff --git
a/activemq-client/src/test/java/org/apache/activemq/util/FactoryFinderTest.java
b/activemq-client/src/test/java/org/apache/activemq/util/FactoryFinderTest.java
index 3e775b2161..b980418452 100644
---
a/activemq-client/src/test/java/org/apache/activemq/util/FactoryFinderTest.java
+++
b/activemq-client/src/test/java/org/apache/activemq/util/FactoryFinderTest.java
@@ -17,18 +17,17 @@
package org.apache.activemq.util;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import java.io.IOException;
-import java.util.List;
import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean;
import org.apache.activemq.transport.TransportFactory;
import org.apache.activemq.transport.nio.NIOTransportFactory;
-import org.apache.activemq.transport.tcp.SslTransport;
import org.apache.activemq.transport.tcp.SslTransportFactory;
import org.apache.activemq.transport.tcp.TcpTransportFactory;
import org.apache.activemq.util.FactoryFinder.ObjectFactory;
@@ -50,12 +49,44 @@ public class FactoryFinderTest {
try {
finder.newInstance("../../tcp");
fail("should have failed instantiation");
+ } catch (InstantiationException e) {
+ assertEquals("Provided key may not contain path separators",
+ e.getMessage());
+ }
+ }
+
+ // Test path traversal attempts will throw an error
+ @Test
+ public void testPathTraversal2() throws Exception {
+ FactoryFinder<TransportFactory> finder
+ = new FactoryFinder<>(TRANSPORT_FACTORY_PATH,
TransportFactory.class, null);
+ assertNull(finder.getAllowedImpls());
+
+ try {
+ finder.newInstance("..");
+ fail("should have failed instantiation");
} catch (InstantiationException e) {
assertEquals("Provided key escapes the FactoryFinder configured
directory",
e.getMessage());
}
}
+ @Test
+ public void testPathTraversal3() throws Exception {
+ FactoryFinder<TransportFactory> finder
+ = new FactoryFinder<>(TRANSPORT_FACTORY_PATH,
TransportFactory.class, null);
+ assertNull(finder.getAllowedImpls());
+
+ // test backslashes as well
+ try {
+ finder.newInstance("..\\..\\tcp");
+ fail("should have failed instantiation");
+ } catch (InstantiationException e) {
+ assertEquals("Provided key may not contain path separators",
+ e.getMessage());
+ }
+ }
+
// WireFormatFactory is not assignable to TransportFactory
// So the constructor should throw an IllegalArgumentException
@Test(expected = IllegalArgumentException.class)
@@ -169,6 +200,8 @@ public class FactoryFinderTest {
assertNull(finder.getAllowedImpls());
assertNotNull(finder.newInstance("tcp"));
assertNotNull(finder.newInstance("ssl"));
+ // should not contain backslashes, even on Windows
+ assertFalse(finder.resolvePath("tcp").contains("\\"));;
try {
// abc is allowed because we are not filtering by allowed impls but
@@ -199,4 +232,17 @@ public class FactoryFinderTest {
assertNotNull(finder.newInstance("ssl"));
assertNotNull(finder.newInstance("nio"));
}
+
+ @Test
+ public void testBackslashReplace() throws Exception {
+ // Forward slashes are usually used but this test uses backslashes to
+ // test that resolvePath() will replace any backslashes
+ String backslashPath = TRANSPORT_FACTORY_PATH.replace("/", "\\");
+ FactoryFinder<TransportFactory> finder = new
FactoryFinder<>(backslashPath,
+ TransportFactory.class, null);
+ assertNotNull(finder.newInstance("tcp"));
+ // should not contain backslashes
+ assertFalse(finder.resolvePath("tcp").contains("\\"));
+ }
+
}
diff --git
a/activemq-stomp/src/test/java/org/apache/activemq/transport/stomp/ProtocolConverterTest.java
b/activemq-stomp/src/test/java/org/apache/activemq/transport/stomp/ProtocolConverterTest.java
index 17d293e3d8..0e6ac096cf 100644
---
a/activemq-stomp/src/test/java/org/apache/activemq/transport/stomp/ProtocolConverterTest.java
+++
b/activemq-stomp/src/test/java/org/apache/activemq/transport/stomp/ProtocolConverterTest.java
@@ -161,7 +161,14 @@ public class ProtocolConverterTest {
converter.loadTranslator("../stomp-test");
fail("should have failed");
} catch (InstantiationException e) {
- assertTrue(e.getMessage().contains("rovided key escapes the
FactoryFinder configured directory"));
+ assertTrue(e.getMessage().contains("Provided key may not contain
path separators"));
+ }
+
+ try {
+ converter.loadTranslator("..");
+ fail("should have failed");
+ } catch (InstantiationException e) {
+ assertTrue(e.getMessage().contains("Provided key escapes the
FactoryFinder configured directory"));
}
// fallback
diff --git
a/activemq-web/src/test/java/org/apache/activemq/web/QueueBrowseServletTest.java
b/activemq-web/src/test/java/org/apache/activemq/web/QueueBrowseServletTest.java
index 8abea6d40a..f46dcfef77 100644
---
a/activemq-web/src/test/java/org/apache/activemq/web/QueueBrowseServletTest.java
+++
b/activemq-web/src/test/java/org/apache/activemq/web/QueueBrowseServletTest.java
@@ -77,6 +77,22 @@ public class QueueBrowseServletTest {
QueueBrowseServlet servlet = new QueueBrowseServlet();
// illegal path traversal
when(request.getParameter("view")).thenReturn("../../simple");
+ try {
+ servlet.getMessageRenderer(request);
+ fail("Should have thrown an exception");
+ } catch (NoSuchViewStyleException e) {
+ Throwable rootCause = e.getRootCause();
+ assertTrue(rootCause instanceof InstantiationException);
+ // view is in allow list but wrong interface type
+ assertEquals(rootCause.getMessage(), "Provided key may not contain
path separators");
+ }
+ }
+
+ @Test
+ public void testPathTraversal2() throws Exception {
+ QueueBrowseServlet servlet = new QueueBrowseServlet();
+ // illegal path traversal
+ when(request.getParameter("view")).thenReturn("..");
try {
servlet.getMessageRenderer(request);
fail("Should have thrown an exception");
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information, visit: https://activemq.apache.org/contact
