This is an automated email from the ASF dual-hosted git repository.
roryqi pushed a commit to branch branch-1.3
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/branch-1.3 by this push:
new b4dd833ca4 [Cherry-pick to branch-1.3] [#12024] improvement(common):
harden JDBC unsafe-parameter detection against connectionProperties smuggling
(#12025) (#12185)
b4dd833ca4 is described below
commit b4dd833ca4686a95a0e270746083284760882f08
Author: YangJie <[email protected]>
AuthorDate: Sat Jul 25 00:45:16 2026 +0800
[Cherry-pick to branch-1.3] [#12024] improvement(common): harden JDBC
unsafe-parameter detection against connectionProperties smuggling (#12025)
(#12185)
### What changes were proposed in this pull request?
- Replace the config-**value** scan in `JdbcUrlUtils` with a
config-**name** scan: every config key, plus each parameter name
embedded in DBCP2's `connectionProperties` value.
- Parse `connectionProperties` exactly as DBCP2 does (`Properties.load`
after replacing `;`→`\n`); reject a malformed value fail-closed.
- Use `Locale.ROOT` for all case folding.
### Why are the changes needed?
`JdbcUrlUtils.validateJdbcConfig` guards against dangerous JDBC
parameters (e.g. MySQL `autoDeserialize`, an RCE enabler) but only
matched config *values* and never inspected DBCP2's
`connectionProperties`. An unsafe parameter could be smuggled to the
driver as a config key, or inside `connectionProperties` (reachable via
`gravitino.bypass.connectionProperties=autoDeserialize=true`, whose
prefix is stripped before the map reaches the datasource), fully
bypassing the check.
Fix: #12024
### Does this PR introduce _any_ user-facing change?
No. A configuration that was already unsafe is now rejected earlier with
a clear error. The exception message wording changes from "...detected
in JDBC URL" to "...detected in JDBC configuration" to reflect that
config keys/`connectionProperties` are inspected too, not only the URL.
### How was this patch tested?
New unit tests in `TestJdbcUrlUtils` (connectionProperties for
MySQL/MariaDB/PostgreSQL, config-key detection, newline / `\uXXXX` /
double-URL-encoding smuggling, whole-name-vs-substring and value-vs-name
negative controls, Turkish-locale folding, malformed-value rejection)
and an end-to-end `TestDataSourceUrlValidation` case through
`DataSourceUtils.createDataSource`. `./gradlew :common:test
:catalogs:catalog-jdbc-common:test -PskipITs` and spotless pass.
---
Cherry-pick of #12025 to `branch-1.3`; replaces the auto-generated
#12183 (its GitHub auto cherry-pick committed conflict markers). The
only manual resolution was in `TestDataSourceUrlValidation.java`, taking
#12025's strengthened cause-message assertions over the older type-only
`assertThrows` on `branch-1.3`.
---
.../jdbc/utils/TestDataSourceUrlValidation.java | 54 ++-
.../org/apache/gravitino/utils/JdbcUrlUtils.java | 83 ++++-
.../apache/gravitino/utils/TestJdbcUrlUtils.java | 402 ++++++++++++++++++++-
3 files changed, 517 insertions(+), 22 deletions(-)
diff --git
a/catalogs/catalog-jdbc-common/src/test/java/org/apache/gravitino/catalog/jdbc/utils/TestDataSourceUrlValidation.java
b/catalogs/catalog-jdbc-common/src/test/java/org/apache/gravitino/catalog/jdbc/utils/TestDataSourceUrlValidation.java
index e05e4f1062..21cdba0984 100644
---
a/catalogs/catalog-jdbc-common/src/test/java/org/apache/gravitino/catalog/jdbc/utils/TestDataSourceUrlValidation.java
+++
b/catalogs/catalog-jdbc-common/src/test/java/org/apache/gravitino/catalog/jdbc/utils/TestDataSourceUrlValidation.java
@@ -53,8 +53,16 @@ public class TestDataSourceUrlValidation {
properties.put(JdbcConfig.USERNAME.getKey(), "test");
properties.put(JdbcConfig.PASSWORD.getKey(), "test");
- Assertions.assertThrows(
- GravitinoRuntimeException.class, () ->
DataSourceUtils.createDataSource(properties));
+ GravitinoRuntimeException gre =
+ Assertions.assertThrows(
+ GravitinoRuntimeException.class, () ->
DataSourceUtils.createDataSource(properties));
+ // createDataSource re-wraps the validation failure, so assert on the
cause to prove the
+ // security check fired (not some unrelated pool/driver error). The
reported name is the
+ // canonical (lower-case) entry from the unsafe-parameter list.
+ Assertions.assertNotNull(gre.getCause());
+ Assertions.assertEquals(
+ "Unsafe MySQL parameter 'allowloadlocalinfile' detected in JDBC
configuration",
+ gre.getCause().getMessage());
}
@Test
@@ -67,8 +75,13 @@ public class TestDataSourceUrlValidation {
properties.put(JdbcConfig.USERNAME.getKey(), "test");
properties.put(JdbcConfig.PASSWORD.getKey(), "test");
- Assertions.assertThrows(
- GravitinoRuntimeException.class, () ->
DataSourceUtils.createDataSource(properties));
+ GravitinoRuntimeException gre =
+ Assertions.assertThrows(
+ GravitinoRuntimeException.class, () ->
DataSourceUtils.createDataSource(properties));
+ Assertions.assertNotNull(gre.getCause());
+ Assertions.assertEquals(
+ "Unsafe PostgreSQL parameter 'socketFactory' detected in JDBC
configuration",
+ gre.getCause().getMessage());
}
@Test
@@ -81,8 +94,37 @@ public class TestDataSourceUrlValidation {
properties.put(JdbcConfig.USERNAME.getKey(), "test");
properties.put(JdbcConfig.PASSWORD.getKey(), "test");
- Assertions.assertThrows(
- GravitinoRuntimeException.class, () ->
DataSourceUtils.createDataSource(properties));
+ GravitinoRuntimeException gre =
+ Assertions.assertThrows(
+ GravitinoRuntimeException.class, () ->
DataSourceUtils.createDataSource(properties));
+ Assertions.assertNotNull(gre.getCause());
+ Assertions.assertEquals(
+ "Unsafe MySQL parameter 'allowloadlocalinfile' detected in JDBC
configuration",
+ gre.getCause().getMessage());
+ }
+
+ @Test
+ public void testRejectMysqlUnsafeParamInConnectionProperties() {
+ // DBCP2 forwards the "connectionProperties" value straight to the JDBC
driver, so an unsafe
+ // MySQL parameter smuggled here must still be rejected. (In the catalog
path a user reaches
+ // this key via the "gravitino.bypass." prefix, which is stripped before
the config map is
+ // handed to the validator.)
+ HashMap<String, String> properties = Maps.newHashMap();
+ properties.put(JdbcConfig.JDBC_DRIVER.getKey(),
"com.mysql.cj.jdbc.Driver");
+ properties.put(JdbcConfig.JDBC_URL.getKey(),
"jdbc:mysql://localhost:3306/test");
+ properties.put(JdbcConfig.USERNAME.getKey(), "test");
+ properties.put(JdbcConfig.PASSWORD.getKey(), "test");
+ properties.put("connectionProperties", "autoDeserialize=true");
+
+ GravitinoRuntimeException gre =
+ Assertions.assertThrows(
+ GravitinoRuntimeException.class, () ->
DataSourceUtils.createDataSource(properties));
+ // createDataSource re-wraps the validation failure, so assert on the
cause to prove the
+ // security check fired (not some unrelated pool/driver error).
+ Assertions.assertNotNull(gre.getCause());
+ Assertions.assertEquals(
+ "Unsafe MySQL parameter 'autoDeserialize' detected in JDBC
configuration",
+ gre.getCause().getMessage());
}
@Test
diff --git a/common/src/main/java/org/apache/gravitino/utils/JdbcUrlUtils.java
b/common/src/main/java/org/apache/gravitino/utils/JdbcUrlUtils.java
index e157109f1b..b451ed7507 100644
--- a/common/src/main/java/org/apache/gravitino/utils/JdbcUrlUtils.java
+++ b/common/src/main/java/org/apache/gravitino/utils/JdbcUrlUtils.java
@@ -20,10 +20,17 @@
package org.apache.gravitino.utils;
import com.google.common.base.Preconditions;
+import java.io.IOException;
+import java.io.StringReader;
import java.net.URLDecoder;
import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashSet;
import java.util.List;
+import java.util.Locale;
import java.util.Map;
+import java.util.Properties;
+import java.util.Set;
import org.apache.commons.lang3.StringUtils;
import org.apache.gravitino.exceptions.GravitinoRuntimeException;
@@ -35,6 +42,11 @@ import
org.apache.gravitino.exceptions.GravitinoRuntimeException;
*/
public class JdbcUrlUtils {
+ // The DBCP2 connection-pool property whose value is a list of connection
properties forwarded
+ // verbatim to the JDBC driver. Unsafe parameters smuggled here would
otherwise evade a check
+ // that only inspects the URL string and the raw config values.
+ private static final String CONNECTION_PROPERTIES_KEY =
"connectionProperties";
+
// Unsafe parameters for MySQL and MariaDB, other parameters like
// trustCertificateKeyStoreUrl, serverTimezone, characterEncoding,
// useSSL are also risky, please use it with caution.
@@ -73,7 +85,7 @@ public class JdbcUrlUtils {
Preconditions.checkArgument(StringUtils.isNotBlank(driver), "Driver class
name can't be blank");
Preconditions.checkArgument(StringUtils.isNotBlank(url), "JDBC URL can't
be blank");
- String lowerUrl = url.toLowerCase();
+ String lowerUrl = url.toLowerCase(Locale.ROOT);
String decodedUrl = recursiveDecode(lowerUrl);
if (decodedUrl.startsWith("jdbc:mysql")) {
@@ -88,13 +100,20 @@ public class JdbcUrlUtils {
private static void checkUnsafeParameters(
String url, Map<String, String> config, List<String> unsafeParams,
String dbType) {
- String lowerUrl = url.toLowerCase();
+ // Percent-decoding in recursiveDecode can reintroduce upper-case
characters (e.g. "%4a" ->
+ // 'J'), so lower-case again here rather than relying on the pre-decode
lower-casing.
+ String lowerUrl = url.toLowerCase(Locale.ROOT);
+
+ // Parameter names that reach the JDBC driver through the config map: the
config keys
+ // themselves (defense in depth) plus any names embedded in the DBCP2
"connectionProperties"
+ // value, which is forwarded verbatim to the driver.
+ Set<String> configParamNames = collectConfigParameterNames(config);
for (String param : unsafeParams) {
- String lowerParam = param.toLowerCase();
- if (lowerUrl.contains(lowerParam) || containsValueIgnoreCase(config,
param)) {
+ String lowerParam = param.toLowerCase(Locale.ROOT);
+ if (lowerUrl.contains(lowerParam) ||
configParamNames.contains(lowerParam)) {
throw new GravitinoRuntimeException(
- "Unsafe %s parameter '%s' detected in JDBC URL", dbType, param);
+ "Unsafe %s parameter '%s' detected in JDBC configuration", dbType,
param);
}
}
}
@@ -116,12 +135,56 @@ public class JdbcUrlUtils {
return decoded;
}
- private static boolean containsValueIgnoreCase(Map<String, String> map,
String value) {
- for (String keyValue : map.values()) {
- if (keyValue != null && keyValue.equalsIgnoreCase(value)) {
- return true;
+ /**
+ * Collects, in lower case, every parameter name that the JDBC driver could
observe via the
+ * configuration map. This includes each config key and each name embedded
in the DBCP2 {@code
+ * connectionProperties} value.
+ */
+ private static Set<String> collectConfigParameterNames(Map<String, String>
config) {
+ Set<String> names = new HashSet<>();
+ if (config == null) {
+ return names;
+ }
+
+ for (Map.Entry<String, String> entry : config.entrySet()) {
+ String key = entry.getKey();
+ if (key == null) {
+ continue;
+ }
+ names.add(key.toLowerCase(Locale.ROOT));
+
+ // DBCP2's BasicDataSourceFactory forwards every name in the
"connectionProperties" value to
+ // the driver. It parses that value by replacing ';' with '\n' and
calling Properties.load,
+ // so we parse it identically here to avoid a parser-differential bypass
(e.g. names split by
+ // embedded newlines or hidden behind '\' / '\\uXXXX' escapes that
Properties.load resolves).
+ if (CONNECTION_PROPERTIES_KEY.equalsIgnoreCase(key)) {
+ for (String name : parseConnectionPropertyNames(entry.getValue())) {
+ names.add(name.toLowerCase(Locale.ROOT));
+ }
}
}
- return false;
+ return names;
+ }
+
+ /**
+ * Parses the property names out of a DBCP2 {@code connectionProperties}
value using the same
+ * semantics DBCP2 itself uses: replace {@code ';'} with a newline and load
as a {@link
+ * Properties} document.
+ */
+ private static Set<String> parseConnectionPropertyNames(String value) {
+ if (value == null) {
+ return Collections.emptySet();
+ }
+ Properties properties = new Properties();
+ try {
+ properties.load(new StringReader(value.replace(';', '\n')));
+ } catch (IOException | IllegalArgumentException e) {
+ // A malformed value cannot be parsed into names to inspect; reject it
defensively rather
+ // than letting an un-inspectable value reach the driver. (DBCP2 parses
it the same way and
+ // would fail too.) The cause is chained for server-side diagnostics but
kept out of the
+ // message to avoid leaking internals to clients.
+ throw new GravitinoRuntimeException(e, "Unable to parse JDBC
connectionProperties");
+ }
+ return properties.stringPropertyNames();
}
}
diff --git
a/common/src/test/java/org/apache/gravitino/utils/TestJdbcUrlUtils.java
b/common/src/test/java/org/apache/gravitino/utils/TestJdbcUrlUtils.java
index f13f1f6316..056178ca72 100644
--- a/common/src/test/java/org/apache/gravitino/utils/TestJdbcUrlUtils.java
+++ b/common/src/test/java/org/apache/gravitino/utils/TestJdbcUrlUtils.java
@@ -20,9 +20,13 @@
package org.apache.gravitino.utils;
import java.util.Collections;
+import java.util.Locale;
import org.apache.gravitino.exceptions.GravitinoRuntimeException;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.parallel.ResourceAccessMode;
+import org.junit.jupiter.api.parallel.ResourceLock;
+import org.junit.jupiter.api.parallel.Resources;
public class TestJdbcUrlUtils {
@@ -70,11 +74,14 @@ public class TestJdbcUrlUtils {
"jdbc:mysql://localhost:0000/test?allowloadlocalinfile=test",
Collections.singletonMap("test", "test")));
Assertions.assertEquals(
- "Unsafe MySQL parameter 'allowloadlocalinfile' detected in JDBC URL",
gre.getMessage());
+ "Unsafe MySQL parameter 'allowloadlocalinfile' detected in JDBC
configuration",
+ gre.getMessage());
}
@Test
public void
whenConfigPropertiesMapContainsUnsafeParam_ShouldThrowGravitinoRuntimeException()
{
+ // The unsafe param appears only as the config KEY (value is unrelated),
so this exercises the
+ // key-based detection rather than incidentally matching on the value.
GravitinoRuntimeException gre =
Assertions.assertThrows(
GravitinoRuntimeException.class,
@@ -82,9 +89,10 @@ public class TestJdbcUrlUtils {
JdbcUrlUtils.validateJdbcConfig(
"testDriver",
"jdbc:mysql://localhost:0000/test",
- Collections.singletonMap("maxAllowedPacket",
"maxAllowedPacket")));
+ Collections.singletonMap("maxAllowedPacket", "1024")));
Assertions.assertEquals(
- "Unsafe MySQL parameter 'maxAllowedPacket' detected in JDBC URL",
gre.getMessage());
+ "Unsafe MySQL parameter 'maxAllowedPacket' detected in JDBC
configuration",
+ gre.getMessage());
}
@Test
@@ -98,7 +106,7 @@ public class TestJdbcUrlUtils {
}
@Test
- public void
whenUnsafeParameterGivenForMariaDB_ShouldThrowGravitintoRuntimeException() {
+ public void
whenUnsafeParameterGivenForMariaDB_ShouldThrowGravitinoRuntimeException() {
GravitinoRuntimeException gre =
Assertions.assertThrows(
GravitinoRuntimeException.class,
@@ -108,7 +116,8 @@ public class TestJdbcUrlUtils {
"jdbc:mariaDB://localhost:0000/test?allowloadlocalinfile=test",
Collections.singletonMap("test", "test")));
Assertions.assertEquals(
- "Unsafe MariaDB parameter 'allowloadlocalinfile' detected in JDBC
URL", gre.getMessage());
+ "Unsafe MariaDB parameter 'allowloadlocalinfile' detected in JDBC
configuration",
+ gre.getMessage());
}
@Test
@@ -132,7 +141,8 @@ public class TestJdbcUrlUtils {
"jdbc:postgresql://localhost:0000/test?socketFactory=test",
Collections.singletonMap("test", "test")));
Assertions.assertEquals(
- "Unsafe PostgreSQL parameter 'socketFactory' detected in JDBC URL",
gre.getMessage());
+ "Unsafe PostgreSQL parameter 'socketFactory' detected in JDBC
configuration",
+ gre.getMessage());
}
@Test
@@ -162,4 +172,384 @@ public class TestJdbcUrlUtils {
IllegalArgumentException.class,
() -> JdbcUrlUtils.validateJdbcConfig("testDriver", "", null));
}
+
+ @Test
+ public void whenConfigIsNullAndUrlIsSafe_ShouldNotThrow() {
+ // A recognized DB URL with a null config map must reach the parameter
check without NPE and
+ // pass (guards the null-config short-circuit in
collectConfigParameterNames).
+ Assertions.assertDoesNotThrow(
+ () ->
+ JdbcUrlUtils.validateJdbcConfig(
+ "testDriver", "jdbc:mysql://localhost:0000/test", null));
+ }
+
+ @Test
+ public void
whenConnectionPropertiesContainsUnsafeParam_ShouldThrowForMySQL() {
+ // DBCP2's BasicDataSourceFactory recognizes a "connectionProperties" key
whose value is a
+ // ';'-delimited list of driver connection properties (e.g.
"k1=v1;k2=v2"). These are passed
+ // straight to the JDBC driver, so an unsafe MySQL parameter smuggled here
bypasses a check
+ // that only inspects the URL string and the raw config values.
+ GravitinoRuntimeException gre =
+ Assertions.assertThrows(
+ GravitinoRuntimeException.class,
+ () ->
+ JdbcUrlUtils.validateJdbcConfig(
+ "testDriver",
+ "jdbc:mysql://localhost:0000/test",
+ Collections.singletonMap("connectionProperties",
"autoDeserialize=true")));
+ Assertions.assertEquals(
+ "Unsafe MySQL parameter 'autoDeserialize' detected in JDBC
configuration",
+ gre.getMessage());
+ }
+
+ @Test
+ public void
whenConnectionPropertiesContainsMultipleUnsafeParams_ShouldThrowForMySQL() {
+ GravitinoRuntimeException gre =
+ Assertions.assertThrows(
+ GravitinoRuntimeException.class,
+ () ->
+ JdbcUrlUtils.validateJdbcConfig(
+ "testDriver",
+ "jdbc:mysql://localhost:0000/test",
+ Collections.singletonMap(
+ "connectionProperties",
+
"useCompression=true;queryInterceptors=com.example.Evil")));
+ Assertions.assertEquals(
+ "Unsafe MySQL parameter 'queryInterceptors' detected in JDBC
configuration",
+ gre.getMessage());
+ }
+
+ @Test
+ public void
whenConnectionPropertiesContainsUnsafeParam_ShouldThrowForPostgreSQL() {
+ GravitinoRuntimeException gre =
+ Assertions.assertThrows(
+ GravitinoRuntimeException.class,
+ () ->
+ JdbcUrlUtils.validateJdbcConfig(
+ "testDriver",
+ "jdbc:postgresql://localhost:0000/test",
+ Collections.singletonMap(
+ "connectionProperties",
"socketFactory=com.example.Evil")));
+ Assertions.assertEquals(
+ "Unsafe PostgreSQL parameter 'socketFactory' detected in JDBC
configuration",
+ gre.getMessage());
+ }
+
+ @Test
+ public void whenUnsafeParamGivenAsConfigKey_ShouldThrowForMySQL() {
+ // Defense in depth: an unsafe parameter supplied directly as a config key
(not embedded in the
+ // URL) must also be rejected.
+ GravitinoRuntimeException gre =
+ Assertions.assertThrows(
+ GravitinoRuntimeException.class,
+ () ->
+ JdbcUrlUtils.validateJdbcConfig(
+ "testDriver",
+ "jdbc:mysql://localhost:0000/test",
+ Collections.singletonMap("autoDeserialize", "true")));
+ Assertions.assertEquals(
+ "Unsafe MySQL parameter 'autoDeserialize' detected in JDBC
configuration",
+ gre.getMessage());
+ }
+
+ @Test
+ public void whenSafeConnectionPropertiesGiven_ShouldNotThrow() {
+ Assertions.assertDoesNotThrow(
+ () ->
+ JdbcUrlUtils.validateJdbcConfig(
+ "testDriver",
+ "jdbc:mysql://localhost:0000/test",
+ Collections.singletonMap(
+ "connectionProperties",
"useCompression=true;connectTimeout=1000")));
+ }
+
+ @Test
+ public void
whenConnectionPropertyNameContainsUnsafeSubstring_ShouldNotThrow() {
+ // Adversarial negative control: a safe name that merely CONTAINS an
unsafe param as a
+ // substring must not be rejected — proves whole-name (not substring)
matching on the config
+ // side.
+ Assertions.assertDoesNotThrow(
+ () ->
+ JdbcUrlUtils.validateJdbcConfig(
+ "testDriver",
+ "jdbc:mysql://localhost:0000/test",
+ Collections.singletonMap("connectionProperties",
"autoDeserializeHelper=true")));
+ }
+
+ @Test
+ public void whenUnsafeParamAppearsOnlyAsConfigValue_ShouldNotThrow() {
+ // The check inspects parameter NAMES (config keys + connectionProperties
names), not values —
+ // only names reach the driver as connection parameters. An unsafe token
appearing solely as a
+ // value under a benign key must not be rejected. Guards against
reintroducing the removed
+ // value-scanning heuristic, which would reject legitimate catalogs.
+ Assertions.assertDoesNotThrow(
+ () ->
+ JdbcUrlUtils.validateJdbcConfig(
+ "testDriver",
+ "jdbc:mysql://localhost:0000/test",
+ Collections.singletonMap("jdbc-password", "autoDeserialize")));
+ }
+
+ @Test
+ public void whenConnectionPropertyValueEqualsUnsafeName_ShouldNotThrow() {
+ // Same principle inside connectionProperties: 'autoDeserialize' as the
VALUE of a benign
+ // property name is harmless (the driver receives name 'foo'), so it must
not be rejected.
+ Assertions.assertDoesNotThrow(
+ () ->
+ JdbcUrlUtils.validateJdbcConfig(
+ "testDriver",
+ "jdbc:mysql://localhost:0000/test",
+ Collections.singletonMap("connectionProperties",
"foo=autoDeserialize")));
+ }
+
+ @Test
+ public void
whenConnectionPropertiesSmugglesUnsafeParamViaNewline_ShouldThrowForMySQL() {
+ // DBCP2 parses the connectionProperties value by replacing ';' with '\n'
and calling
+ // Properties.load, so an embedded newline starts a new property. A parser
that split only on
+ // ';' would miss the second name; parsing exactly as DBCP2 does catches
it.
+ GravitinoRuntimeException gre =
+ Assertions.assertThrows(
+ GravitinoRuntimeException.class,
+ () ->
+ JdbcUrlUtils.validateJdbcConfig(
+ "testDriver",
+ "jdbc:mysql://localhost:0000/test",
+ Collections.singletonMap(
+ "connectionProperties",
"foo=bar\nautoDeserialize=true")));
+ Assertions.assertEquals(
+ "Unsafe MySQL parameter 'autoDeserialize' detected in JDBC
configuration",
+ gre.getMessage());
+ }
+
+ @Test
+ public void
whenConnectionPropertiesSmugglesUnsafeParamViaUnicodeEscape_ShouldThrowForMySQL()
{
+ // "\\u0061utoDeserialize" is decoded to "autoDeserialize" by
Properties.load. A naive
+ // substring parser would see the literal "\\u0061..." and miss it.
+ GravitinoRuntimeException gre =
+ Assertions.assertThrows(
+ GravitinoRuntimeException.class,
+ () ->
+ JdbcUrlUtils.validateJdbcConfig(
+ "testDriver",
+ "jdbc:mysql://localhost:0000/test",
+ Collections.singletonMap(
+ "connectionProperties",
"\\u0061utoDeserialize=true")));
+ Assertions.assertEquals(
+ "Unsafe MySQL parameter 'autoDeserialize' detected in JDBC
configuration",
+ gre.getMessage());
+ }
+
+ @Test
+ public void
whenConnectionPropertiesKeyHasDifferentCase_ShouldThrowForMySQL() {
+ GravitinoRuntimeException gre =
+ Assertions.assertThrows(
+ GravitinoRuntimeException.class,
+ () ->
+ JdbcUrlUtils.validateJdbcConfig(
+ "testDriver",
+ "jdbc:mysql://localhost:0000/test",
+ Collections.singletonMap("ConnectionProperties",
"autoDeserialize=true")));
+ Assertions.assertEquals(
+ "Unsafe MySQL parameter 'autoDeserialize' detected in JDBC
configuration",
+ gre.getMessage());
+ }
+
+ @Test
+ public void
whenConnectionPropertiesHasWhitespaceAroundPairs_ShouldThrowForMySQL() {
+ GravitinoRuntimeException gre =
+ Assertions.assertThrows(
+ GravitinoRuntimeException.class,
+ () ->
+ JdbcUrlUtils.validateJdbcConfig(
+ "testDriver",
+ "jdbc:mysql://localhost:0000/test",
+ Collections.singletonMap(
+ "connectionProperties", "useCompression=true;
autoDeserialize = true")));
+ Assertions.assertEquals(
+ "Unsafe MySQL parameter 'autoDeserialize' detected in JDBC
configuration",
+ gre.getMessage());
+ }
+
+ @Test
+ public void
whenConnectionPropertiesContainsUnsafeParam_ShouldThrowForMariaDB() {
+ GravitinoRuntimeException gre =
+ Assertions.assertThrows(
+ GravitinoRuntimeException.class,
+ () ->
+ JdbcUrlUtils.validateJdbcConfig(
+ "testDriver",
+ "jdbc:mariadb://localhost:0000/test",
+ Collections.singletonMap("connectionProperties",
"autoDeserialize=true")));
+ Assertions.assertEquals(
+ "Unsafe MariaDB parameter 'autoDeserialize' detected in JDBC
configuration",
+ gre.getMessage());
+ }
+
+ @Test
+ public void whenUnsafeParamGivenAsConfigKey_ShouldThrowForPostgreSQL() {
+ GravitinoRuntimeException gre =
+ Assertions.assertThrows(
+ GravitinoRuntimeException.class,
+ () ->
+ JdbcUrlUtils.validateJdbcConfig(
+ "testDriver",
+ "jdbc:postgresql://localhost:0000/test",
+ Collections.singletonMap("socketFactory",
"com.example.Evil")));
+ Assertions.assertEquals(
+ "Unsafe PostgreSQL parameter 'socketFactory' detected in JDBC
configuration",
+ gre.getMessage());
+ }
+
+ @Test
+ public void whenConnectionPropertiesHasTrailingSeparators_ShouldStillThrow()
{
+ GravitinoRuntimeException gre =
+ Assertions.assertThrows(
+ GravitinoRuntimeException.class,
+ () ->
+ JdbcUrlUtils.validateJdbcConfig(
+ "testDriver",
+ "jdbc:mysql://localhost:0000/test",
+ Collections.singletonMap("connectionProperties",
"autoDeserialize=true;;")));
+ Assertions.assertEquals(
+ "Unsafe MySQL parameter 'autoDeserialize' detected in JDBC
configuration",
+ gre.getMessage());
+ }
+
+ @Test
+ public void
whenConnectionPropertiesValueIsMalformed_ShouldThrowUnableToParse() {
+ // A "\\uZZZZ" escape makes Properties.load throw
IllegalArgumentException; the validator
+ // rejects the un-inspectable value rather than letting it reach the
driver (DBCP2 parses it
+ // the same way and would also fail).
+ GravitinoRuntimeException gre =
+ Assertions.assertThrows(
+ GravitinoRuntimeException.class,
+ () ->
+ JdbcUrlUtils.validateJdbcConfig(
+ "testDriver",
+ "jdbc:mysql://localhost:0000/test",
+ Collections.singletonMap("connectionProperties",
"\\uZZZZ=true")));
+ Assertions.assertEquals("Unable to parse JDBC connectionProperties",
gre.getMessage());
+ // The parse cause is chained for server-side diagnostics; guard against a
"simplification"
+ // that drops it.
+ Assertions.assertInstanceOf(IllegalArgumentException.class,
gre.getCause());
+ }
+
+ @Test
+ public void whenUnsafeParamGivenAsConfigKey_ShouldThrowForMariaDB() {
+ GravitinoRuntimeException gre =
+ Assertions.assertThrows(
+ GravitinoRuntimeException.class,
+ () ->
+ JdbcUrlUtils.validateJdbcConfig(
+ "testDriver",
+ "jdbc:mariadb://localhost:0000/test",
+ Collections.singletonMap("autoDeserialize", "true")));
+ Assertions.assertEquals(
+ "Unsafe MariaDB parameter 'autoDeserialize' detected in JDBC
configuration",
+ gre.getMessage());
+ }
+
+ @Test
+ public void whenPercentEscapeDecodesToUpperCaseParam_ShouldThrowForMySQL() {
+ // "%41" decodes to uppercase 'A', so after recursiveDecode the URL
contains
+ // "AllowLoadLocalInfile". The check must re-lower-case the decoded URL
(not rely on the
+ // pre-decode lower-casing) to still match the unsafe parameter.
+ GravitinoRuntimeException gre =
+ Assertions.assertThrows(
+ GravitinoRuntimeException.class,
+ () ->
+ JdbcUrlUtils.validateJdbcConfig(
+ "testDriver",
+
"jdbc:mysql://localhost:0000/test?%41llowLoadLocalInfile=true",
+ Collections.singletonMap("test", "test")));
+ Assertions.assertEquals(
+ "Unsafe MySQL parameter 'allowloadlocalinfile' detected in JDBC
configuration",
+ gre.getMessage());
+ }
+
+ @Test
+ public void whenParamIsDoubleUrlEncoded_ShouldThrowForMySQL() {
+ // "%2561" is a double-encoding of 'a': one decode pass yields "%61", a
second yields 'a'. The
+ // check must decode recursively (not a single pass) to collapse nested
encoding before
+ // matching, otherwise the unsafe parameter slips through.
+ GravitinoRuntimeException gre =
+ Assertions.assertThrows(
+ GravitinoRuntimeException.class,
+ () ->
+ JdbcUrlUtils.validateJdbcConfig(
+ "testDriver",
+
"jdbc:mysql://localhost:0000/test?%2561llowLoadLocalInfile=true",
+ Collections.singletonMap("test", "test")));
+ Assertions.assertEquals(
+ "Unsafe MySQL parameter 'allowloadlocalinfile' detected in JDBC
configuration",
+ gre.getMessage());
+ }
+
+ @Test
+ @ResourceLock(value = Resources.LOCALE, mode = ResourceAccessMode.READ_WRITE)
+ public void whenDefaultLocaleIsTurkish_ShouldStillMatchParamWithI() {
+ // The attacker-supplied form carries a capital 'I'
('statementInterceptors'). Under the
+ // Turkish locale a default-locale toLowerCase() folds 'I' to a dotless
'ı', while Locale.ROOT
+ // folds it to an ASCII dotted 'i' to match the unsafe-param list entry.
Using the capital-'I'
+ // input at each site means a Locale.ROOT -> toLowerCase() regression at
ANY of the four
+ // input-lowercasing sites (URL pre-decode, URL post-decode, config key,
connectionProperties
+ // name) diverges from the ROOT-folded param and is caught here — a
default-locale CI would not
+ // otherwise catch it.
+ Locale previous = Locale.getDefault();
+ try {
+ Locale.setDefault(Locale.forLanguageTag("tr"));
+
+ // URL path — literal capital 'I' is folded at the pre-decode site.
+ Assertions.assertEquals(
+ "Unsafe MySQL parameter 'statementInterceptors' detected in JDBC
configuration",
+ Assertions.assertThrows(
+ GravitinoRuntimeException.class,
+ () ->
+ JdbcUrlUtils.validateJdbcConfig(
+ "testDriver",
+
"jdbc:mysql://localhost:0000/test?statementInterceptors=com.example.Evil",
+ Collections.singletonMap("test", "test")))
+ .getMessage());
+
+ // URL path — '%49' decodes to 'I' only AFTER recursiveDecode, so this
exercises the
+ // post-decode re-lowercasing site specifically.
+ Assertions.assertEquals(
+ "Unsafe MySQL parameter 'statementInterceptors' detected in JDBC
configuration",
+ Assertions.assertThrows(
+ GravitinoRuntimeException.class,
+ () ->
+ JdbcUrlUtils.validateJdbcConfig(
+ "testDriver",
+
"jdbc:mysql://localhost:0000/test?statement%49nterceptors=com.example.Evil",
+ Collections.singletonMap("test", "test")))
+ .getMessage());
+
+ // Config-key path.
+ Assertions.assertEquals(
+ "Unsafe MySQL parameter 'statementInterceptors' detected in JDBC
configuration",
+ Assertions.assertThrows(
+ GravitinoRuntimeException.class,
+ () ->
+ JdbcUrlUtils.validateJdbcConfig(
+ "testDriver",
+ "jdbc:mysql://localhost:0000/test",
+ Collections.singletonMap("statementInterceptors",
"com.example.Evil")))
+ .getMessage());
+
+ // connectionProperties-name path.
+ Assertions.assertEquals(
+ "Unsafe MySQL parameter 'statementInterceptors' detected in JDBC
configuration",
+ Assertions.assertThrows(
+ GravitinoRuntimeException.class,
+ () ->
+ JdbcUrlUtils.validateJdbcConfig(
+ "testDriver",
+ "jdbc:mysql://localhost:0000/test",
+ Collections.singletonMap(
+ "connectionProperties",
"statementInterceptors=com.example.Evil")))
+ .getMessage());
+ } finally {
+ Locale.setDefault(previous);
+ }
+ }
}
