You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jackrabbit.apache.org by kw...@apache.org on 2026/10/15 07:32:22 UTC
(jackrabbit-filevault) 06/06: JCRVLT-862 Prevent loading arbitrary classes in config.xml
This is an automated email from the ASF dual-hosted git repository.
kwin pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/jackrabbit-filevault.git
commit 77ce15a860e0a418b6a38e1d56c87336940fd694
Author: Konrad Windszus <kw...@apache.org>
AuthorDate: Fri Aug 21 20:20:54 2026 +0200
JCRVLT-862 Prevent loading arbitrary classes in config.xml
Attribute "class" in element "include"/"exclude" was previously
instantiated unconditionally.
---
src/site/markdown/config.md | 2 +-
.../jackrabbit/vault/fs/config/ConfigHelper.java | 46 ++++++++---
.../vault/fs/config/VaultFsConfig11.java | 4 +-
.../jackrabbit/vault/fs/config/package-info.java | 2 +-
.../vault/fs/config/ConfigHelperTest.java | 92 ++++++++++++++++++++++
5 files changed, 131 insertions(+), 15 deletions(-)
diff --git a/src/site/markdown/config.md b/src/site/markdown/config.md
index 91594537..90fe61e3 100644
--- a/src/site/markdown/config.md
+++ b/src/site/markdown/config.md
@@ -75,7 +75,7 @@ Type | Used Class | Description
Every type except for the first can be restricted via `matches` and `contains` child elements. The former applies a given filter to the current item, while the latter must match ones of the children.
Both contain arbitrarily many `include` or `exclude` nodes.
-Each have either an element `class` for a fully qualified class name or consists of one of the following attributes (which implicitly sets a class)
+Each has either an attribute `class` specifying a fully qualified class name (implementing `org.apache.jackrabbit.vault.fs.api.ItemFilter`) or consists of one of the following attributes (which implicitly sets a class)
Filter attribute | Values | Additional attributes | Used Filter Class | Description
--- | --- | --- | --- | ---
diff --git a/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/config/ConfigHelper.java b/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/config/ConfigHelper.java
index 30dd3ec9..b5376e84 100644
--- a/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/config/ConfigHelper.java
+++ b/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/config/ConfigHelper.java
@@ -65,6 +65,28 @@ public class ConfigHelper {
}
public Object create(Element elem) throws ConfigurationException {
+ return create(elem, Object.class);
+ }
+
+ /**
+ * Creates an instance for the given config element, requiring the configured class to be of
+ * the given expected type. Configurations are frequently read from untrusted sources (e.g.
+ * {@code META-INF/vault/config.xml} inside a content package, parsed when the package is
+ * opened), so the class named by the {@code class} attribute must never be instantiated -
+ * and its static initializer therefore never triggered - unless it is of the type the
+ * caller will actually use. For the same reason the historic {@code Class#FIELD} static
+ * field access syntax is rejected: reading an arbitrary static field of an arbitrary class
+ * is not a configuration mechanism but a code-execution primitive.
+ *
+ * @param elem the config element
+ * @param expectedType the type the configured class must be assignable to
+ * @return the new instance (or the element's text content if no class is configured)
+ * @throws ConfigurationException if the class cannot be resolved, is not assignable to
+ * {@code expectedType}, uses the rejected {@code #} field syntax, or cannot be
+ * instantiated
+ * @since 2.10.0 (Bundle version 4.3.0)
+ */
+ public Object create(Element elem, Class<?> expectedType) throws ConfigurationException {
String className = elem.getAttribute("class");
if (className == null || className.equals("")) {
@@ -74,13 +96,11 @@ public class ConfigHelper {
// create string object
return elem.getFirstChild().getNodeValue();
}
- String field = null;
- int pos = className.indexOf('#');
- if (pos > 0) {
- field = className.substring(pos + 1);
- className = className.substring(0, pos);
- }
+ if (className.indexOf('#') >= 0) {
+ throw new ConfigurationException("Static field access syntax in class attribute of " + elem.getNodeName()
+ + " is not allowed: " + className);
+ }
// try to get class without prepending package
Class<?> clazz = null;
try {
@@ -110,15 +130,17 @@ public class ConfigHelper {
throw new ConfigurationException("Error while creating instance for " + elem.getNodeName(), e);
}
}
+ // verify the type BEFORE instantiation: loading a class does not run its static
+ // initializer, but constructing it does - so an attacker-supplied class name must
+ // never reach newInstance() unless it is of the expected type
+ if (!expectedType.isAssignableFrom(clazz)) {
+ throw new ConfigurationException("Class " + clazz.getName() + " configured for " + elem.getNodeName()
+ + " is not of expected type " + expectedType.getName());
+ }
try {
- if (field == null) {
- return clazz.getDeclaredConstructor().newInstance();
- } else {
- return clazz.getField(field).get(null);
- }
+ return clazz.getDeclaredConstructor().newInstance();
} catch (InstantiationException
| IllegalAccessException
- | NoSuchFieldException
| IllegalArgumentException
| InvocationTargetException
| NoSuchMethodException
diff --git a/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/config/VaultFsConfig11.java b/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/config/VaultFsConfig11.java
index 8831ad5c..d6af1161 100644
--- a/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/config/VaultFsConfig11.java
+++ b/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/config/VaultFsConfig11.java
@@ -131,7 +131,9 @@ class VaultFsConfig11 extends AbstractVaultFsConfig {
ItemFilter filter = null;
String clazz = child.getAttribute("class");
if (clazz != null && clazz.length() > 0) {
- filter = (ItemFilter) helper.create(child);
+ // config.xml may stem from an untrusted package (parsed at archive open):
+ // require the configured class to be an ItemFilter before instantiating it
+ filter = (ItemFilter) helper.create(child, ItemFilter.class);
} else {
// create filter based on some attributes
if (attrs.getNamedItem("nodeType") != null) {
diff --git a/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/config/package-info.java b/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/config/package-info.java
index a29b5cbf..5794b22a 100644
--- a/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/config/package-info.java
+++ b/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/config/package-info.java
@@ -15,7 +15,7 @@
* limitations under the License.
*/
-@Version("2.9.0")
+@Version("2.10.0")
package org.apache.jackrabbit.vault.fs.config;
import org.osgi.annotation.versioning.Version;
diff --git a/vault-core/src/test/java/org/apache/jackrabbit/vault/fs/config/ConfigHelperTest.java b/vault-core/src/test/java/org/apache/jackrabbit/vault/fs/config/ConfigHelperTest.java
new file mode 100644
index 00000000..98f956ee
--- /dev/null
+++ b/vault-core/src/test/java/org/apache/jackrabbit/vault/fs/config/ConfigHelperTest.java
@@ -0,0 +1,92 @@
+/*
+ * 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.jackrabbit.vault.fs.config;
+
+import javax.xml.parsers.DocumentBuilderFactory;
+
+import java.util.concurrent.atomic.AtomicBoolean;
+
+import org.apache.jackrabbit.vault.fs.api.ItemFilter;
+import org.apache.jackrabbit.vault.fs.filter.IsNodeFilter;
+import org.junit.Test;
+import org.w3c.dom.Document;
+import org.w3c.dom.Element;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+/**
+ * Tests that {@link ConfigHelper#create(Element, Class)} never instantiates (and thereby
+ * never statically initializes) a configured class that is not of the expected type, and
+ * refuses the {@code Class#FIELD} static field access syntax. The class attribute may stem
+ * from an untrusted package's {@code META-INF/vault/config.xml}.
+ */
+public class ConfigHelperTest {
+
+ /**
+ * Set by {@link Canary}'s static initializer; kept outside the canary class so it can be
+ * read without triggering that initializer.
+ */
+ static final AtomicBoolean CANARY_INITIALIZED = new AtomicBoolean();
+
+ /** Not an {@link ItemFilter}; records whether it was ever statically initialized. */
+ public static class Canary {
+ static {
+ CANARY_INITIALIZED.set(true);
+ }
+ }
+
+ private static Element element(String name, String className) throws Exception {
+ Document doc = DocumentBuilderFactory.newInstance().newDocumentBuilder().newDocument();
+ Element elem = doc.createElement(name);
+ elem.setAttribute("class", className);
+ return elem;
+ }
+
+ @Test
+ public void testItemFilterClassIsCreated() throws Exception {
+ ConfigHelper helper = new ConfigHelper();
+ Object o = helper.create(element("include", IsNodeFilter.class.getName()), ItemFilter.class);
+ assertTrue(o instanceof IsNodeFilter);
+ }
+
+ @Test
+ public void testNonItemFilterClassIsRefusedBeforeInstantiation() throws Exception {
+ ConfigHelper helper = new ConfigHelper();
+ try {
+ helper.create(element("include", Canary.class.getName()), ItemFilter.class);
+ fail("a configured class not implementing ItemFilter must be refused");
+ } catch (ConfigurationException e) {
+ // expected
+ }
+ assertFalse("the refused class must not have been statically initialized", CANARY_INITIALIZED.get());
+ }
+
+ @Test
+ public void testStaticFieldAccessSyntaxIsRefused() throws Exception {
+ ConfigHelper helper = new ConfigHelper();
+ try {
+ helper.create(element("include", "java.lang.System#out"), ItemFilter.class);
+ fail("the 'Class#FIELD' syntax must be refused");
+ } catch (ConfigurationException e) {
+ // expected
+ }
+ }
+}