This is an automated email from the ASF dual-hosted git repository.
zabetak pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/calcite.git
The following commit(s) were added to refs/heads/main by this push:
new a6051a6e00 [CALCITE-7713] Add allowlist option in ClassNameFilter
a6051a6e00 is described below
commit a6051a6e00b200a15a4b7f28bebd6a0a2a5b3384
Author: Stamatis Zampetakis <[email protected]>
AuthorDate: Wed Jun 24 19:53:28 2026 +0200
[CALCITE-7713] Add allowlist option in ClassNameFilter
---
.github/workflows/main.yml | 8 ++
build.gradle.kts | 1 +
.../calcite/config/CalciteSystemProperty.java | 19 +++++
.../org/apache/calcite/model/ClassNameFilter.java | 89 +++++++++-------------
.../apache/calcite/model/ClassNameFilterTest.java | 89 ++++++++++++++++++++++
.../org/apache/calcite/model/ModelHandlerTest.java | 25 ++----
site/_docs/history.md | 5 ++
sqlsh | 3 +-
sqlsh.bat | 4 +-
9 files changed, 170 insertions(+), 73 deletions(-)
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 165f2752ab..df77ec38ee 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -71,6 +71,8 @@ jobs:
remote-build-cache-proxy-enabled: false
arguments: --scan --no-parallel --no-daemon build javadoc
- name: 'sqlline and sqllsh'
+ env:
+ JAVA_OPTS: "-Dcalcite.model.classes.allowed=org.apache."
shell: cmd
run: |
call sqlline.bat -e '!quit'
@@ -101,6 +103,8 @@ jobs:
remote-build-cache-proxy-enabled: false
arguments: --scan --no-parallel --no-daemon build
- name: 'sqlline and sqllsh'
+ env:
+ JAVA_OPTS: "-Dcalcite.model.classes.allowed=org.apache."
shell: cmd
run: |
call sqlline.bat -e '!quit'
@@ -131,6 +135,8 @@ jobs:
remote-build-cache-proxy-enabled: false
arguments: --scan --no-parallel --no-daemon build
- name: 'sqlline and sqllsh'
+ env:
+ JAVA_OPTS: "-Dcalcite.model.classes.allowed=org.apache."
shell: cmd
run: |
call sqlline.bat -e '!quit'
@@ -324,6 +330,8 @@ jobs:
remote-build-cache-proxy-enabled: false
arguments: --scan --no-parallel --no-daemon build javadoc
- name: 'sqlline and sqllsh'
+ env:
+ JAVA_OPTS: "-Dcalcite.model.classes.allowed=org.apache."
run: |
./sqlline -e '!quit'
echo
diff --git a/build.gradle.kts b/build.gradle.kts
index 44e00c7fa3..4a45616720 100644
--- a/build.gradle.kts
+++ b/build.gradle.kts
@@ -915,6 +915,7 @@ fun passProperty(name: String, default: String? = null) {
passProperty("user.timezone", "UTC")
passProperty("calcite.avatica.version",
props.string("calcite.avatica.version"))
passProperty("gradle.rootDir", rootDir.toString())
+ systemProperty("calcite.model.classes.allowed",
"org.,java.lang.")
val props = System.getProperties()
for (e in props.propertyNames() as
`java.util`.Enumeration<String>) {
if (e.startsWith("calcite.") || e.startsWith("avatica.")) {
diff --git
a/core/src/main/java/org/apache/calcite/config/CalciteSystemProperty.java
b/core/src/main/java/org/apache/calcite/config/CalciteSystemProperty.java
index b0efb1a05d..39ba5c2f32 100644
--- a/core/src/main/java/org/apache/calcite/config/CalciteSystemProperty.java
+++ b/core/src/main/java/org/apache/calcite/config/CalciteSystemProperty.java
@@ -455,6 +455,25 @@ public final class CalciteSystemProperty<T> {
public static final CalciteSystemProperty<Integer>
JOIN_SELECTOR_COMPACT_CODE_THRESHOLD =
intProperty("calcite.join.selector.compact.code.threshold", 100);
+ /**
+ * Comma-separated allowlist of class-name patterns that may be loaded
+ * by reflection from a Calcite model (user-defined functions, custom
+ * schemas/tables, JDBC drivers, dialect factories, lattice statistic
+ * providers).
+ *
+ * <p>By default, the allowlist is empty and class loading is fully disabled.
+ * When non-empty, a class name must match the allowlist in addition to
+ * clearing the denylist.
+ *
+ * <p>Pattern syntax: a pattern ending in {@code "."} matches any class
+ * in that package or its sub-packages; otherwise the pattern matches a
+ * class name exactly.
+ *
+ * @see org.apache.calcite.model.ModelHandler
+ */
+ public static final CalciteSystemProperty<String> MODEL_CLASSES_ALLOWED =
+ stringProperty("calcite.model.classes.allowed", "");
+
/**
* Comma-separated patterns to add to the built-in denylist of class
* names that may not be loaded by reflection from a Calcite model
diff --git a/core/src/main/java/org/apache/calcite/model/ClassNameFilter.java
b/core/src/main/java/org/apache/calcite/model/ClassNameFilter.java
index 5a0a948cdc..0c37c2c5cb 100644
--- a/core/src/main/java/org/apache/calcite/model/ClassNameFilter.java
+++ b/core/src/main/java/org/apache/calcite/model/ClassNameFilter.java
@@ -20,34 +20,25 @@
import com.google.common.collect.ImmutableList;
+import org.apiguardian.api.API;
import org.checkerframework.checker.nullness.qual.Nullable;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
-import java.util.function.Predicate;
/**
* Filters class names that may be loaded by reflection from a Calcite
* model: user-defined functions, custom schemas, custom tables, JDBC
* drivers, dialect factories, and lattice statistic providers.
*
- * <p>{@link #standard()} returns the filter applied by
- * {@link ModelHandler}: the built-in {@link #DEFAULT_DENYLIST} together
- * with any patterns from
- * {@link CalciteSystemProperty#MODEL_CLASSES_DENIED} (which
- * <em>extends</em> the denylist).
- *
- * <p>The denylist is a comma-separated pattern string. A pattern ending
- * in {@code "."} matches any class in that package or its sub-packages;
+ * <p>The behavior of the filter is determined by the allowlist and the
denylist.
+ * Both lists are comma separated patterns determining a package or class name.
+ * A pattern ending in {@code "."} matches any class in that package or its
sub-packages;
* otherwise the pattern matches a class name exactly. Whitespace around
* commas is ignored.
- *
- * <p>The denylist is not a sandbox. Any string passed to a
- * {@code className}, {@code factory}, {@code jdbcDriver},
- * {@code sqlDialectFactory}, or {@code statisticProvider} field is
- * classpath-equivalent; only accept models from trusted sources.
*/
-class ClassNameFilter implements Predicate<String> {
+@API(since = "1.43.0", status = API.Status.EXPERIMENTAL)
+public final class ClassNameFilter {
/** Built-in denylist: class-name patterns known to enable RCE when
* registered as UDFs, schema/table factories, JDBC drivers, dialect
* factories, or lattice statistic providers. */
@@ -75,74 +66,68 @@ class ClassNameFilter implements Predicate<String> {
+ "jdk.internal.";
/** Cache shared by all factory calls; filters are immutable and small,
- * so identical denylist inputs need only be parsed once. */
+ * so identical (denylist, allowlist) inputs need only be parsed once. */
private static final ConcurrentMap<String, ClassNameFilter> CACHE =
new ConcurrentHashMap<>();
/** The standard filter, built once from the built-in denylist plus
- * the {@link CalciteSystemProperty#MODEL_CLASSES_DENIED} extension.
- * Initialized via {@link #of} so it shares the same cache. */
+ * the {@link CalciteSystemProperty} pair. Initialized via {@link #of}
+ * so it shares the same cache. */
private static final ClassNameFilter STANDARD =
of(
append(DEFAULT_DENYLIST,
- CalciteSystemProperty.MODEL_CLASSES_DENIED.value()));
+ CalciteSystemProperty.MODEL_CLASSES_DENIED.value()),
+ CalciteSystemProperty.MODEL_CLASSES_ALLOWED.value());
private final ImmutableList<String> denylist;
+ private final ImmutableList<String> allowlist;
- private ClassNameFilter(String denylist) {
+ private ClassNameFilter(String denylist, String allowlist) {
this.denylist = parse(denylist);
+ this.allowlist = parse(allowlist);
}
- /** Returns the standard filter used by {@link ModelHandler}: the
+ /** Returns the standard filter used by {@link ModelHandler}. The
* built-in {@link #DEFAULT_DENYLIST} (extended by
- * {@link CalciteSystemProperty#MODEL_CLASSES_DENIED}). */
+ * {@link CalciteSystemProperty#MODEL_CLASSES_DENIED}) plus the
+ * allowlist from
+ * {@link CalciteSystemProperty#MODEL_CLASSES_ALLOWED}. */
static ClassNameFilter standard() {
return STANDARD;
}
- /** Returns a filter parsed from a comma-separated denylist pattern
- * string; may be empty. Filters are cached, so repeated calls with
- * the same argument return the same instance. */
- static ClassNameFilter of(String denylist) {
- return CACHE.computeIfAbsent(denylist, ClassNameFilter::new);
+ /** Returns a filter parsed from comma-separated {@code denylist} and
+ * {@code allowlist} pattern strings; either may be empty. Filters are
+ * cached, so repeated calls with the same arguments return the same
+ * instance. */
+ public static ClassNameFilter of(String denylist, String allowlist) {
+ // NUL is forbidden in JVM class names, so concatenating with NUL is
+ // an injection-proof cache key.
+ String key = denylist + '\0' + allowlist;
+ return CACHE.computeIfAbsent(key,
+ k -> new ClassNameFilter(denylist, allowlist));
}
- /** Returns whether {@code classRef} is allowed (not on the denylist).
- * A null reference is allowed.
- *
- * <p>{@code classRef} may be a plain class name or the
- * {@code "ClassName#STATIC_FIELD"} form accepted by
- * {@link org.apache.calcite.avatica.AvaticaUtils#instantiatePlugin};
- * the field portion is stripped before matching. */
- @Override public boolean test(@Nullable String classRef) {
+ /** Throws {@link SecurityException} if {@code classRef} is not allowed
+ * by this filter. A null reference is a no-op. */
+ void check(@Nullable String classRef) {
if (classRef == null) {
- return true;
+ return;
}
String className = stripFieldRef(classRef);
for (String pattern : denylist) {
if (matches(pattern, className)) {
- return false;
+ throw new SecurityException(
+ "Class '" + className + "' rejected by the denylist (pattern '" +
pattern + "').");
}
}
- return true;
- }
- /** Throws {@link SecurityException} if {@code classRef} is on the
- * denylist. A null reference is a no-op. */
- void check(@Nullable String classRef) {
- if (classRef == null) {
- return;
- }
- String className = stripFieldRef(classRef);
- for (String pattern : denylist) {
+ for (String pattern : allowlist) {
if (matches(pattern, className)) {
- throw new SecurityException("Class '" + className
- + "' is rejected by the Calcite class-name filter "
- + "(matches denylist pattern '" + pattern + "'). "
- + "If this load is unintended, adjust the model; the "
- + "denylist cannot be loosened at runtime.");
+ return;
}
}
+ throw new SecurityException("Class '" + className + "' rejected by the
allowlist.");
}
private static String stripFieldRef(String classRef) {
diff --git
a/core/src/test/java/org/apache/calcite/model/ClassNameFilterTest.java
b/core/src/test/java/org/apache/calcite/model/ClassNameFilterTest.java
new file mode 100644
index 0000000000..be459d9611
--- /dev/null
+++ b/core/src/test/java/org/apache/calcite/model/ClassNameFilterTest.java
@@ -0,0 +1,89 @@
+/*
+ * 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.calcite.model;
+
+import org.junit.jupiter.api.Test;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsString;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+/**
+ * Unit tests for {@link ClassNameFilter}.
+ */
+public class ClassNameFilterTest {
+ @Test void testDefaultProdValuesForDenyAllowList() {
+ // This represents the default prod configuration of the project
+ // where the system properties both default to empty and
+ // basically every class is rejected mainly due to the empty allowlist
+ ClassNameFilter cf = ClassNameFilter.of("", "");
+ assertThrows(SecurityException.class, () -> cf.check("java.lang.String"));
+ assertThrows(SecurityException.class,
+ () -> cf.check("org.apache.calcite.adapter.jdbc.JdbcSchema$Factory"));
+ }
+
+ @Test void testAllowListWithSinglePackagePattern() {
+ ClassNameFilter cf = ClassNameFilter.of("", "org.");
+ assertThrows(SecurityException.class, () -> cf.check("java.lang.String"));
+ assertDoesNotThrow(() ->
cf.check("org.apache.calcite.adapter.jdbc.JdbcSchema$Factory"));
+ }
+
+ @Test void testAllowListWithSinglePackageNoPattern() {
+ ClassNameFilter cf = ClassNameFilter.of("", "org");
+ assertThrows(SecurityException.class, () -> cf.check("java.lang.String"));
+ assertThrows(SecurityException.class,
+ () -> cf.check("org.apache.calcite.adapter.jdbc.JdbcSchema$Factory"));
+ }
+
+ @Test void testAllowListWithMultiplePackagePatterns() {
+ ClassNameFilter cf = ClassNameFilter.of("", "org.,java.");
+ assertDoesNotThrow(() -> cf.check("java.lang.String"));
+ assertDoesNotThrow(() ->
cf.check("org.apache.calcite.adapter.jdbc.JdbcSchema$Factory"));
+ assertThrows(SecurityException.class, () ->
cf.check("com.sun.media.sound.Toolkit"));
+ }
+
+ @Test void testAllowListWithSingleClass() {
+ ClassNameFilter cf = ClassNameFilter.of("", "java.lang.String");
+ assertDoesNotThrow(() -> cf.check("java.lang.String"));
+ assertThrows(SecurityException.class,
+ () -> cf.check("org.apache.calcite.adapter.jdbc.JdbcSchema$Factory"));
+ assertThrows(SecurityException.class, () -> cf.check("java.lang.Math"));
+ }
+
+ @Test void testAllowListWithMultipleClasses() {
+ ClassNameFilter cf = ClassNameFilter.of("",
"java.lang.String,java.lang.Math");
+ assertDoesNotThrow(() -> cf.check("java.lang.String"));
+ assertThrows(SecurityException.class, () ->
cf.check("java.lang.StringBuffer"));
+ assertThrows(SecurityException.class,
+ () -> cf.check("org.apache.calcite.adapter.jdbc.JdbcSchema$Factory"));
+ assertDoesNotThrow(() -> cf.check("java.lang.Math"));
+ }
+
+ @Test void testStandardFilter() {
+ // Note that test specific system properties are in effect
+ ClassNameFilter cf = ClassNameFilter.standard();
+ assertDoesNotThrow(() -> cf.check("java.lang.String"));
+ assertDoesNotThrow(() ->
cf.check("org.apache.calcite.adapter.jdbc.JdbcSchema$Factory"));
+ SecurityException x1 =
+ assertThrows(SecurityException.class, () ->
cf.check("com.sun.media.sound.Toolkit"));
+ assertThat(x1.getMessage(), containsString("rejected by the allowlist"));
+ SecurityException x2 =
+ assertThrows(SecurityException.class, () ->
cf.check("javax.naming.InitialContext"));
+ assertThat(x2.getMessage(), containsString("rejected by the denylist"));
+ }
+}
diff --git a/core/src/test/java/org/apache/calcite/model/ModelHandlerTest.java
b/core/src/test/java/org/apache/calcite/model/ModelHandlerTest.java
index 4cebda1454..438ec79c3c 100644
--- a/core/src/test/java/org/apache/calcite/model/ModelHandlerTest.java
+++ b/core/src/test/java/org/apache/calcite/model/ModelHandlerTest.java
@@ -31,7 +31,6 @@
import java.sql.DriverManager;
import java.util.Properties;
import java.util.Set;
-import java.util.function.Predicate;
import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.CoreMatchers.is;
@@ -39,6 +38,7 @@
import static org.hamcrest.CoreMatchers.notNullValue;
import static org.hamcrest.CoreMatchers.sameInstance;
import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static java.util.Objects.requireNonNull;
@@ -82,7 +82,7 @@ public class ModelHandlerTest {
SchemaPlus root = CalciteSchema.createRootSchema(false, false).plus();
// java.lang.String is not in the standard denylist; the custom
// filter denies the whole java.lang. package.
- ClassNameFilter strict = ClassNameFilter.of("java.lang.");
+ ClassNameFilter strict = ClassNameFilter.of("java.lang.", "java.");
String model = "inline:{"
+ " version: '1.0',"
+ " defaultSchema: 'X',"
@@ -164,31 +164,20 @@ public class ModelHandlerTest {
"org.apache.calcite.adapter.jdbc.JdbcSchema$Factory#INSTANCE");
}
- @Test void testPredicateContract() {
- // ClassNameFilter implements Predicate<String>: true means "allowed".
- Predicate<String> filter = ClassNameFilter.standard();
- assertThat(filter.test(null), is(true));
- assertThat(filter.test("javax.naming.InitialContext"), is(false));
- assertThat(filter.test("java.lang.Runtime#getRuntime"), is(false));
- assertThat(
- filter.test(
- "org.apache.calcite.adapter.jdbc.JdbcSchema$Factory"), is(true));
- }
-
@Test void testFactoryMethodsCacheInstances() {
// standard() returns a single cached instance.
assertThat(ClassNameFilter.standard(),
sameInstance(ClassNameFilter.standard()));
// of() returns the same instance for equal inputs.
- ClassNameFilter a = ClassNameFilter.of("com.evil.");
- ClassNameFilter b = ClassNameFilter.of("com.evil.");
+ ClassNameFilter a = ClassNameFilter.of("com.evil.", "javax.");
+ ClassNameFilter b = ClassNameFilter.of("com.evil.", "javax.");
assertThat(a, sameInstance(b));
// Different inputs produce different instances.
- ClassNameFilter c = ClassNameFilter.of("com.evil.,com.example.");
+ ClassNameFilter c = ClassNameFilter.of("com.evil.,com.example.", "javax.");
assertThat(a, not(sameInstance(c)));
// The cached filter behaves as configured.
- assertThat(a.test("com.evil.Payload"), is(false));
- assertThat(a.test("javax.naming.InitialContext"), is(true));
+ assertThrows(SecurityException.class, () -> a.check("com.evil.Payload"));
+ assertDoesNotThrow(() -> a.check("javax.naming.InitialContext"));
}
@Test void testAppendCombinesPatternStrings() {
diff --git a/site/_docs/history.md b/site/_docs/history.md
index c72d6f78d9..9520e7d05f 100644
--- a/site/_docs/history.md
+++ b/site/_docs/history.md
@@ -54,6 +54,11 @@ #### Breaking Changes
filter evaluation now run in Java, and the `arrow-gandiva` dependency is no
longer included in the Arrow module or BOM.
+* [<a
href="https://issues.apache.org/jira/browse/CALCITE-7713";>CALCITE-7713</a>]
+Class loading from model files has been disabled by default. Any attempt to
load
+classes from model files will lead to `SecurityException` unless an appropriate
+pattern is set in `calcite.model.classes.allowed` system property.
+
#### New features
{: #new-features-1-43-0}
diff --git a/sqlsh b/sqlsh
index 437060a249..18c9d305a3 100755
--- a/sqlsh
+++ b/sqlsh
@@ -43,6 +43,7 @@ if [ "x$CACHE_SQLLINE_CLASSPATH" != "xY" ] || [ ! -f "$CP" ];
then
fi
VM_OPTS=
-JAVA_OPTS="-Djavax.xml.parsers.DocumentBuilderFactory=com.sun.org.apache.xerces.internal.jaxp.DocumentBuilderFactoryImpl
${JAVA_OPTS}"
+# Running sqlsh is explicitly for OS adapter, so we can set the allowed
classes to avoid security exception when loading the model file.
+JAVA_OPTS="-Djavax.xml.parsers.DocumentBuilderFactory=com.sun.org.apache.xerces.internal.jaxp.DocumentBuilderFactoryImpl
-Dcalcite.model.classes.allowed=org.apache.calcite.adapter.os. ${JAVA_OPTS}"
exec java $VM_OPTS -cp "${CP}" $JAVA_OPTS
org.apache.calcite.adapter.os.SqlShell "$@"
diff --git a/sqlsh.bat b/sqlsh.bat
index 2ad45c5439..ec4bed1816 100644
--- a/sqlsh.bat
+++ b/sqlsh.bat
@@ -31,7 +31,7 @@ if not defined CACHE_SQLLINE_CLASSPATH (
if exist "%CP%" del "%CP%"
)
if not exist "%CP%" (call "%DIRNAME%\gradlew" --console plain -q
:buildSqllineClasspath)
-
-set
JAVA_OPTS=-Djavax.xml.parsers.DocumentBuilderFactory=com.sun.org.apache.xerces.internal.jaxp.DocumentBuilderFactoryImpl
%JAVA_OPTS%
+:: Running sqlsh is explicitly for OS adapter, so we can set the allowed
classes to avoid security exception when loading the model file.
+set
JAVA_OPTS=-Djavax.xml.parsers.DocumentBuilderFactory=com.sun.org.apache.xerces.internal.jaxp.DocumentBuilderFactoryImpl
-Dcalcite.model.classes.allowed=org.apache.calcite.adapter.os. %JAVA_OPTS%
java -Xmx1g -cp "%CP%" %JAVA_OPTS% org.apache.calcite.adapter.os.SqlShell %*
