Skip to content

Commit e7cd23a

Browse files
[bugfix] fix a remote command execution. (apache#3250)
1 parent 81ad7d5 commit e7cd23a

File tree

1 file changed

+36
-10
lines changed
  • hertzbeat-collector/hertzbeat-collector-basic/src/main/java/org/apache/hertzbeat/collector/collect/database

1 file changed

+36
-10
lines changed

hertzbeat-collector/hertzbeat-collector-basic/src/main/java/org/apache/hertzbeat/collector/collect/database/JdbcCommonCollect.java

+36-10
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,25 @@ public class JdbcCommonCollect extends AbstractCollect {
6363

6464
private static final String[] VULNERABLE_KEYWORDS = {"allowLoadLocalInfile", "allowLoadLocalInfileInPath", "useLocalInfile"};
6565

66+
private static final String[] BLACK_LIST = {
67+
// dangerous SQL commands - may cause database structure damage or data leakage
68+
"create trigger", "create alias", "runscript from", "shutdown", "drop table",
69+
"drop database", "create function", "alter system", "grant all", "revoke all",
70+
71+
// file IO related - may cause server files to be read or written
72+
"allowloadlocalinfile", "allowloadlocalinfileinpath", "uselocalinfile",
73+
74+
// code execution related - may result in remote code execution
75+
"init=", "javaobjectserializer=", "runscript", "serverstatusdiffinterceptor",
76+
"queryinterceptors=", "statementinterceptors=", "exceptioninterceptors=",
77+
78+
// multiple statement execution - may lead to SQL injection
79+
"allowmultiqueries",
80+
81+
// deserialization related - may result in remote code execution
82+
"autodeserialize", "detectcustomcollations",
83+
};
84+
6685
private final GlobalConnectionCache connectionCommonCache = GlobalConnectionCache.getInstance();
6786

6887

@@ -331,17 +350,24 @@ private String constructDatabaseUrl(JdbcProtocol jdbcProtocol, String host, Stri
331350
if (Objects.nonNull(jdbcProtocol.getUrl())
332351
&& !Objects.equals("", jdbcProtocol.getUrl())
333352
&& jdbcProtocol.getUrl().startsWith("jdbc")) {
334-
// convert the URL to lowercase for case-insensitive checking
335-
String url = jdbcProtocol.getUrl().toLowerCase();
336-
// check whether the parameter is valid
337-
if (url.contains("create trigger") || url.contains("create alias") || url.contains("runscript from")
338-
|| url.contains("allowloadlocalinfile") || url.contains("allowloadlocalinfileinpath")
339-
|| url.contains("uselocalinfile") || url.contains("autodeserialize") || url.contains("detectcustomcollations")
340-
|| url.contains("serverstatusdiffinterceptor")) {
341-
throw new IllegalArgumentException("Invalid JDBC URL: contains malicious characters.");
353+
// limit url length
354+
if (jdbcProtocol.getUrl().length() > 2048) {
355+
throw new IllegalArgumentException("JDBC URL length exceeds maximum limit of 2048 characters");
356+
}
357+
// remove special characters
358+
String cleanedUrl = jdbcProtocol.getUrl().replaceAll("[\\x00-\\x1F\\x7F]", "");
359+
String url = cleanedUrl.toLowerCase();
360+
// backlist check
361+
for (String keyword : BLACK_LIST) {
362+
if (url.contains(keyword)) {
363+
throw new IllegalArgumentException("Invalid JDBC URL: contains potentially malicious parameter: " + keyword);
364+
}
365+
}
366+
// url format check
367+
if (!url.matches("^jdbc:[a-zA-Z0-9]+://[^\\s]+$")) {
368+
throw new IllegalArgumentException("Invalid JDBC URL format");
342369
}
343-
// when has config jdbc url, use it
344-
return jdbcProtocol.getUrl();
370+
return cleanedUrl;
345371
}
346372
return switch (jdbcProtocol.getPlatform()) {
347373
case "mysql", "mariadb" -> "jdbc:mysql://" + host + ":" + port

0 commit comments

Comments
 (0)