Skip to content

Commit ac1d642

Browse files
authored
GH-16425 Add JDBC parameter validation [nocheck] (#16432)
* GH-16425 Add JDBC parameter validation * swap exception expectation
1 parent dc7bfa7 commit ac1d642

File tree

2 files changed

+65
-0
lines changed

2 files changed

+65
-0
lines changed

h2o-core/src/main/java/water/jdbc/SQLManager.java

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,20 @@
55
import water.parser.ParseDataset;
66
import water.util.Log;
77

8+
import java.io.UnsupportedEncodingException;
9+
import java.net.URI;
10+
import java.net.URISyntaxException;
11+
import java.net.URLDecoder;
812
import java.sql.*;
13+
import java.util.Arrays;
14+
import java.util.List;
915
import java.util.Objects;
1016
import java.util.concurrent.ArrayBlockingQueue;
1117
import java.util.concurrent.atomic.AtomicLong;
18+
import java.util.regex.Matcher;
19+
import java.util.regex.Pattern;
20+
import java.util.stream.Collectors;
21+
import java.util.stream.Stream;
1222

1323
public class SQLManager {
1424

@@ -30,6 +40,15 @@ public class SQLManager {
3040

3141
private static final String TMP_TABLE_ENABLED = H2O.OptArgs.SYSTEM_PROP_PREFIX + "sql.tmp_table.enabled";
3242

43+
private static final String DISALLOWED_JDBC_PARAMETERS_PARAM = H2O.OptArgs.SYSTEM_PROP_PREFIX + "sql.jdbc.disallowed.parameters";
44+
45+
private static final Pattern JDBC_PARAMETERS_REGEX_PATTERN = Pattern.compile("(?i)[?;&]([a-z]+)=");
46+
47+
private static final List<String> DEFAULT_JDBC_DISALLOWED_PARAMETERS = Stream.of(
48+
"autoDeserialize", "queryInterceptors", "allowLoadLocalInfile", "allowMultiQueries", //mysql
49+
"allowLoadLocalInfileInPath", "allowUrlInLocalInfile", "allowPublicKeyRetrieval", //mysql
50+
"init", "script", "shutdown" //h2
51+
).map(String::toLowerCase).collect(Collectors.toList());
3352
private static AtomicLong NEXT_TABLE_NUM = new AtomicLong(0);
3453

3554
static Key<Frame> nextTableKey(String prefix, String postfix) {
@@ -58,6 +77,7 @@ public static Job<Frame> importSqlTable(
5877
final String username, final String password, final String columns,
5978
final Boolean useTempTable, final String tempTableName,
6079
final SqlFetchMode fetchMode, final Integer numChunksHint) {
80+
validateJdbcUrl(connection_url);
6181

6282
final Key<Frame> destination_key = nextTableKey(table, "sql_to_hex");
6383
final Job<Frame> j = new Job<>(destination_key, Frame.class.getName(), "Import SQL Table");
@@ -533,6 +553,7 @@ private static int estimateConcurrentConnections(final int cloudSize, final shor
533553
* @throws SQLException if a database access error occurs or the url is
534554
*/
535555
public static Connection getConnectionSafe(String url, String username, String password) throws SQLException {
556+
validateJdbcUrl(url);
536557
initializeDatabaseDriver(getDatabaseType(url));
537558
try {
538559
return DriverManager.getConnection(url, username, password);
@@ -588,6 +609,30 @@ static void initializeDatabaseDriver(String databaseType) {
588609
}
589610
}
590611

612+
public static void validateJdbcUrl(String jdbcUrl) throws IllegalArgumentException {
613+
if (jdbcUrl == null || jdbcUrl.trim().isEmpty()) {
614+
throw new IllegalArgumentException("JDBC URL is null or empty");
615+
}
616+
617+
if (!jdbcUrl.toLowerCase().startsWith("jdbc:")) {
618+
throw new IllegalArgumentException("JDBC URL must start with 'jdbc:'");
619+
}
620+
621+
Matcher matcher = JDBC_PARAMETERS_REGEX_PATTERN.matcher(jdbcUrl);
622+
String property = System.getProperty(DISALLOWED_JDBC_PARAMETERS_PARAM);
623+
List<String> disallowedParameters = property == null ?
624+
DEFAULT_JDBC_DISALLOWED_PARAMETERS :
625+
Arrays.stream(property.split(",")).map(String::toLowerCase).collect(Collectors.toList());
626+
627+
while (matcher.find()) {
628+
String key = matcher.group(1);
629+
if (disallowedParameters.contains(key.toLowerCase())) {
630+
throw new IllegalArgumentException("Potentially dangerous JDBC parameter found: " + key +
631+
". That behavior can be altered by setting " + DISALLOWED_JDBC_PARAMETERS_PARAM + " env variable to another comma separated list.");
632+
}
633+
}
634+
}
635+
591636
static class SqlTableToH2OFrameStreaming {
592637
final String _table, _columns, _databaseType;
593638
final int _numCol;

h2o-core/src/test/java/water/jdbc/SQLManagerTest.java

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,4 +145,24 @@ public void testBuildSelectChunkSql() {
145145
Assert.assertEquals("SELECT * FROM mytable LIMIT 1310 OFFSET 0",
146146
SQLManager.buildSelectChunkSql("", "mytable", 0, 1310, "*", null));
147147
}
148+
149+
@Test
150+
public void testValidateJdbcConnectionStringH2() {
151+
exception.expect(IllegalArgumentException.class);
152+
exception.expectMessage("Potentially dangerous JDBC parameter found: init");
153+
154+
String h2MaliciousJdbc = "jdbc:h2:mem:test;MODE=MSSQLServer;init=CREATE ALIAS RBT AS '@groovy.transform.ASTTest(value={ assert java.lang.Runtime.getRuntime().exec(\"reboot\")" + "})" + "def rbt" + "'";
155+
156+
SQLManager.validateJdbcUrl(h2MaliciousJdbc);
157+
}
158+
159+
@Test
160+
public void testValidateJdbcConnectionStringMysql() {
161+
exception.expect(IllegalArgumentException.class);
162+
exception.expectMessage("Potentially dangerous JDBC parameter found: autoDeserialize");
163+
164+
String mysqlMaliciousJdbc = "jdbc:mysql://domain:123/test?autoDeserialize=true&queryInterceptors=com.mysql.cj.jdbc.interceptors.ServerStatusDiffInterceptor&user=abcd";
165+
166+
SQLManager.validateJdbcUrl(mysqlMaliciousJdbc);
167+
}
148168
}

0 commit comments

Comments
 (0)