This is an automated email from the ASF dual-hosted git repository.
yuqi1129 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new c2fc58bf13 [#11839] fix(clickhouse): escape single quotes in database
and table name SQL to prevent injection (#11840)
c2fc58bf13 is described below
commit c2fc58bf13b618d6169f47f6cbc24c873b4c2144
Author: weijiajun <[email protected]>
AuthorDate: Fri Jul 3 20:58:45 2026 +0800
[#11839] fix(clickhouse): escape single quotes in database and table name
SQL to prevent injection (#11840)
### What changes were proposed in this pull request?
- Override `generateDatabaseExistSql` in `ClickHouseDatabaseOperations`
to query `system.databases` with proper single-quote escaping via
`escapeSingleQuotes`. The inherited default targets
`information_schema.SCHEMATA` which ClickHouse does not support.
- Escape `databaseName` and `tableName` in
`ClickHouseTableOperations.getIndexes()` before interpolating into
`QUERY_INDEXES_SQL`.
- Add unit tests for `generateDatabaseExistSql` covering normal names
and names containing single quotes.
### Why are the changes needed?
Both code paths interpolate user-provided names into SQL string literals
without escaping single quotes, creating SQL injection vulnerabilities.
ClickHouse's JDBC driver does not support `PreparedStatement`
parameterization for `system`-table queries, so string escaping
(single-quote doubling) is the correct and only viable mitigation. The
fix reuses the existing `ClickHouseClusterUtils.escapeSingleQuotes()`
utility already used by `generateCreateDatabaseSql`,
`generateAlterTableSql`, and other methods in the same module.
Fix: #11839
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
- `./gradlew :catalogs-contrib:catalog-jdbc-clickhouse:spotlessApply` —
passes
- `./gradlew :catalogs-contrib:catalog-jdbc-clickhouse:build` — passes
- `./gradlew :catalogs-contrib:catalog-jdbc-clickhouse:test` — all tests
pass, including new tests:
- `testGenerateDatabaseExistSqlNormalName` — verifies normal name
produces correct SQL
- `testGenerateDatabaseExistSqlSingleQuoteEscaped` — verifies `test'db`
→ `test''db`
---
.../operations/ClickHouseDatabaseOperations.java | 9 +++
.../operations/ClickHouseTableOperations.java | 4 +-
.../TestClickHouseDatabaseOperations.java | 18 +++++
.../TestClickHouseTableOperationsUnit.java | 87 ++++++++++++++++++++++
4 files changed, 117 insertions(+), 1 deletion(-)
diff --git
a/catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/operations/ClickHouseDatabaseOperations.java
b/catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/operations/ClickHouseDatabaseOperations.java
index 0d4ba9b7ea..e35946364d 100644
---
a/catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/operations/ClickHouseDatabaseOperations.java
+++
b/catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/operations/ClickHouseDatabaseOperations.java
@@ -66,6 +66,15 @@ public class ClickHouseDatabaseOperations extends
JdbcDatabaseOperations {
return CLICK_HOUSE_SYSTEM_DATABASES;
}
+ @Override
+ protected String generateDatabaseExistSql(String databaseName) {
+ // Escape single quotes to prevent SQL injection.
+ // JdbcDatabaseOperations#exist executes this query via Statement (no
parameters), so we must
+ // escape values embedded in string literals.
+ String escaped = escapeSingleQuotes(databaseName);
+ return String.format("SELECT name FROM system.databases WHERE name =
'%s'", escaped);
+ }
+
@Override
public List<String> listDatabases() {
List<String> databaseNames = new ArrayList<>();
diff --git
a/catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/operations/ClickHouseTableOperations.java
b/catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/operations/ClickHouseTableOperations.java
index e3cd9378e1..6eb48dcea0 100644
---
a/catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/operations/ClickHouseTableOperations.java
+++
b/catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/operations/ClickHouseTableOperations.java
@@ -111,7 +111,9 @@ public class ClickHouseTableOperations extends
JdbcTableOperations {
protected List<Index> getIndexes(Connection connection, String databaseName,
String tableName) {
// cause clickhouse not impl getPrimaryKeys yet, ref:
// https://github.com/ClickHouse/clickhouse-java/issues/1625
- String sql = QUERY_INDEXES_SQL.formatted(databaseName, tableName);
+ String sql =
+ QUERY_INDEXES_SQL.formatted(
+ escapeSingleQuotes(databaseName), escapeSingleQuotes(tableName));
try (PreparedStatement preparedStatement =
connection.prepareStatement(sql);
ResultSet resultSet = preparedStatement.executeQuery()) {
diff --git
a/catalogs-contrib/catalog-jdbc-clickhouse/src/test/java/org/apache/gravitino/catalog/clickhouse/operations/TestClickHouseDatabaseOperations.java
b/catalogs-contrib/catalog-jdbc-clickhouse/src/test/java/org/apache/gravitino/catalog/clickhouse/operations/TestClickHouseDatabaseOperations.java
index e728b43736..4c39226912 100644
---
a/catalogs-contrib/catalog-jdbc-clickhouse/src/test/java/org/apache/gravitino/catalog/clickhouse/operations/TestClickHouseDatabaseOperations.java
+++
b/catalogs-contrib/catalog-jdbc-clickhouse/src/test/java/org/apache/gravitino/catalog/clickhouse/operations/TestClickHouseDatabaseOperations.java
@@ -200,4 +200,22 @@ public class TestClickHouseDatabaseOperations {
Mockito.verify(connection).setCatalog("information_schema");
Mockito.verify(statement).executeUpdate("CREATE DATABASE `new_db`");
}
+
+ //
---------------------------------------------------------------------------
+ // generateDatabaseExistSql — SQL injection escape
+ //
---------------------------------------------------------------------------
+
+ @Test
+ void testGenerateDatabaseExistSqlNormalName() {
+ TestableClickHouseDatabaseOperations ops = newOps();
+ String sql = ops.generateDatabaseExistSql("my_db");
+ Assertions.assertEquals("SELECT name FROM system.databases WHERE name =
'my_db'", sql);
+ }
+
+ @Test
+ void testGenerateDatabaseExistSqlSingleQuoteEscaped() {
+ TestableClickHouseDatabaseOperations ops = newOps();
+ String sql = ops.generateDatabaseExistSql("test'db");
+ Assertions.assertEquals("SELECT name FROM system.databases WHERE name =
'test''db'", sql);
+ }
}
diff --git
a/catalogs-contrib/catalog-jdbc-clickhouse/src/test/java/org/apache/gravitino/catalog/clickhouse/operations/TestClickHouseTableOperationsUnit.java
b/catalogs-contrib/catalog-jdbc-clickhouse/src/test/java/org/apache/gravitino/catalog/clickhouse/operations/TestClickHouseTableOperationsUnit.java
new file mode 100644
index 0000000000..e31362c782
--- /dev/null
+++
b/catalogs-contrib/catalog-jdbc-clickhouse/src/test/java/org/apache/gravitino/catalog/clickhouse/operations/TestClickHouseTableOperationsUnit.java
@@ -0,0 +1,87 @@
+/*
+ * 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.gravitino.catalog.clickhouse.operations;
+
+import java.sql.Connection;
+import java.sql.PreparedStatement;
+import java.sql.ResultSet;
+import java.util.HashMap;
+import java.util.List;
+import
org.apache.gravitino.catalog.clickhouse.converter.ClickHouseColumnDefaultValueConverter;
+import
org.apache.gravitino.catalog.clickhouse.converter.ClickHouseExceptionConverter;
+import
org.apache.gravitino.catalog.clickhouse.converter.ClickHouseTypeConverter;
+import org.apache.gravitino.rel.indexes.Index;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.mockito.ArgumentCaptor;
+import org.mockito.Mockito;
+
+public class TestClickHouseTableOperationsUnit {
+
+ private static final class ExposedClickHouseTableOperations extends
ClickHouseTableOperations {
+ List<Index> callGetIndexes(Connection connection, String databaseName,
String tableName)
+ throws Exception {
+ return getIndexes(connection, databaseName, tableName);
+ }
+ }
+
+ private ExposedClickHouseTableOperations newOps() {
+ ExposedClickHouseTableOperations ops = new
ExposedClickHouseTableOperations();
+ ops.initialize(
+ null,
+ new ClickHouseExceptionConverter(),
+ new ClickHouseTypeConverter(),
+ new ClickHouseColumnDefaultValueConverter(),
+ new HashMap<>());
+ return ops;
+ }
+
+ //
---------------------------------------------------------------------------
+ // getIndexes — SQL injection escape
+ //
---------------------------------------------------------------------------
+
+ @Test
+ void testGetIndexesSqlEscapesSingleQuotes() throws Exception {
+ ExposedClickHouseTableOperations ops = newOps();
+
+ PreparedStatement primaryKeyStmt = Mockito.mock(PreparedStatement.class);
+ ResultSet primaryKeyRs = Mockito.mock(ResultSet.class);
+ PreparedStatement secondaryStmt = Mockito.mock(PreparedStatement.class);
+ ResultSet secondaryRs = Mockito.mock(ResultSet.class);
+
+ Mockito.when(primaryKeyRs.next()).thenReturn(false);
+ Mockito.when(primaryKeyStmt.executeQuery()).thenReturn(primaryKeyRs);
+ Mockito.when(secondaryRs.next()).thenReturn(false);
+ Mockito.when(secondaryStmt.executeQuery()).thenReturn(secondaryRs);
+
+ Connection connection = Mockito.mock(Connection.class);
+ ArgumentCaptor<String> sqlCaptor = ArgumentCaptor.forClass(String.class);
+ Mockito.when(connection.prepareStatement(sqlCaptor.capture()))
+ .thenReturn(primaryKeyStmt)
+ .thenReturn(secondaryStmt);
+
+ ops.callGetIndexes(connection, "db'1", "t'1");
+
+ // First captured SQL is the primary-key QUERY_INDEXES_SQL
(string-interpolated).
+ String primaryKeySql = sqlCaptor.getAllValues().get(0);
+ Assertions.assertTrue(
+ primaryKeySql.contains("db''1"), "database single quote should be
doubled");
+ Assertions.assertTrue(primaryKeySql.contains("t''1"), "table single quote
should be doubled");
+ }
+}
