diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index a972de65e..7f3ad51ef 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -7,6 +7,7 @@ ### Updated ### Fixed +- Fixed SQL injection vulnerability in binary parameter handling. - Fixed `setCatalog()` and `setSchema()` producing invalid SQL (e.g. `SET CATALOG ``name``) when the catalog or schema name was passed already wrapped in backticks. Backticks are now stripped before wrapping, and `getCatalog()`/`getSchema()` return the bare identifier name. - Fixed metadata SQL generation for catalog, schema, and table identifiers containing backticks. - Fixed SEA result truncation when direct results are disabled. Large, highly-compressible results that span multiple chunks were delivered inline via the old hybrid path and truncated to the first chunk. The SQL Execution path now uses an async (`0s`) wait timeout when direct results are disabled, so results are returned via external links and fetched in full. diff --git a/src/main/java/com/databricks/jdbc/common/util/SQLInterpolator.java b/src/main/java/com/databricks/jdbc/common/util/SQLInterpolator.java index 1928dfe26..54f7dd112 100644 --- a/src/main/java/com/databricks/jdbc/common/util/SQLInterpolator.java +++ b/src/main/java/com/databricks/jdbc/common/util/SQLInterpolator.java @@ -6,8 +6,13 @@ import com.databricks.jdbc.exception.DatabricksValidationException; import com.databricks.jdbc.model.core.ColumnInfoTypeName; import java.util.Map; +import java.util.regex.Pattern; public class SQLInterpolator { + + // Hex literal emitted by setBytes (X''); only this shape is spliced unquoted (SEC-20590). + private static final Pattern HEX_LITERAL_PATTERN = Pattern.compile("X'[0-9A-Fa-f]*'"); + protected static String escapeInputs(String input) { if (input == null) return null; StringBuilder out = new StringBuilder(input.length() + 16); @@ -50,8 +55,12 @@ private static String formatObject(ImmutableSqlParameter object) { if (object == null || object.value() == null) { return NULL_STRING; } else if (object.type() == ColumnInfoTypeName.BINARY) { - // Don't wrap within quotes. Don't treat hex literals as string. - return object.value().toString(); + // Splice raw only if it's a genuine hex literal; otherwise escape to avoid SQL injection. + String binaryValue = object.value().toString(); + if (HEX_LITERAL_PATTERN.matcher(binaryValue).matches()) { + return binaryValue; + } + return escapeInputs(binaryValue); } else if (object.value() instanceof String) { return escapeInputs(object.value().toString()); } else if (object.type() == ColumnInfoTypeName.TIMESTAMP diff --git a/src/test/java/com/databricks/jdbc/common/util/SQLInterpolatorTest.java b/src/test/java/com/databricks/jdbc/common/util/SQLInterpolatorTest.java index 22b35f465..2abb5ed5c 100644 --- a/src/test/java/com/databricks/jdbc/common/util/SQLInterpolatorTest.java +++ b/src/test/java/com/databricks/jdbc/common/util/SQLInterpolatorTest.java @@ -94,6 +94,23 @@ public void testBinaryType() throws DatabricksValidationException { assertEquals(expected, SQLInterpolator.interpolateSQL(sql, params)); } + // SEC-20590: a non-hex String typed as BINARY must be escaped, not executed as SQL. + @Test + public void testBinaryTypeRejectsNonHexString() throws DatabricksValidationException { + String sql = "SELECT ?"; + Map params = new HashMap<>(); + params.put(1, getSqlParam(1, "current_user()", DatabricksTypeUtil.BINARY)); + assertEquals("SELECT 'current_user()'", SQLInterpolator.interpolateSQL(sql, params)); + } + + @Test + public void testBinaryTypeEscapesMaliciousHexLookalike() throws DatabricksValidationException { + String sql = "SELECT ?"; + Map params = new HashMap<>(); + params.put(1, getSqlParam(1, "X'41' OR 1=1 --", DatabricksTypeUtil.BINARY)); + assertEquals("SELECT 'X''41'' OR 1=1 --'", SQLInterpolator.interpolateSQL(sql, params)); + } + @Test public void testTimestampType() throws DatabricksValidationException { String sql = "INSERT INTO events (id, created_at) VALUES (?, ?)";