You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@iotdb.apache.org by "Young-Leo (via GitHub)" <gi...@apache.org> on 2025/11/11 12:01:35 UTC
[PR] Support the JDBC client to prevent SQL injection [iotdb]
Young-Leo opened a new pull request, #16734:
URL: https://github.com/apache/iotdb/pull/16734
Support the JDBC client to prevent SQL injection
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support the JDBC client to prevent SQL injection [iotdb]
Posted by "JackieTien97 (via GitHub)" <gi...@apache.org>.
JackieTien97 merged PR #16734:
URL: https://github.com/apache/iotdb/pull/16734
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support the JDBC client to prevent SQL injection [iotdb]
Posted by "JackieTien97 (via GitHub)" <gi...@apache.org>.
JackieTien97 commented on code in PR #16734:
URL: https://github.com/apache/iotdb/pull/16734#discussion_r2517244762
##########
iotdb-client/jdbc/src/main/java/org/apache/iotdb/jdbc/IoTDBPreparedStatement.java:
##########
@@ -909,19 +909,21 @@ public void setShort(int parameterIndex, short x) throws SQLException {
@Override
public void setString(int parameterIndex, String x) {
- // if the sql is an insert statement and the value is not a string literal, add single quotes
- // The table model only supports single quotes, the tree model sql both single and double quotes
- if ("table".equalsIgnoreCase(getSqlDialect())
- || ((sql.trim().toUpperCase().startsWith("INSERT")
- && !((x.startsWith("'") && x.endsWith("'"))
- || ((x.startsWith("\"") && x.endsWith("\""))
- && "tree".equals(getSqlDialect())))))) {
- this.parameters.put(parameterIndex, "'" + x + "'");
+ if (x == null) {
+ this.parameters.put(parameterIndex, null);
} else {
- this.parameters.put(parameterIndex, x);
+ this.parameters.put(parameterIndex, "'" + escapeSingleQuotes(x) + "'");
}
}
+ private String escapeSingleQuotes(String value) {
+ if (value == null) {
+ return null;
Review Comment:
throw Exception? You've already do null check in caller.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support the JDBC client to prevent SQL injection [iotdb]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #16734:
URL: https://github.com/apache/iotdb/pull/16734#discussion_r2516510380
##########
iotdb-client/jdbc/src/test/java/org/apache/iotdb/jdbc/IoTDBPreparedStatementTest.java:
##########
@@ -400,4 +400,141 @@ public void testInsertStatement4() throws Exception {
"INSERT INTO root.ln.wf01.wt02(time,a,b,c,d,e,f) VALUES(2020-01-01T10:10:10,false,123,123234345,123.423,-1323.0,'abc')",
argument.getValue().getStatement());
}
+
+ // ========== Table Model SQL Injection Prevention Tests ==========
+
+ @SuppressWarnings("resource")
+ @Test
+ public void testTableModelLoginInjectionWithComment() throws Exception {
+ // Login interface SQL injection attack 1: Using -- comments to bypass password checks
+ when(connection.getSqlDialect()).thenReturn("table");
+ String sql = "SELECT * FROM users WHERE username = ? AND password = ?";
+ IoTDBPreparedStatement ps =
+ new IoTDBPreparedStatement(connection, client, sessionId, sql, zoneId);
+ ps.setString(1, "admin' --");
+ ps.setString(2, "password");
+ ps.execute();
+
+ ArgumentCaptor<TSExecuteStatementReq> argument =
+ ArgumentCaptor.forClass(TSExecuteStatementReq.class);
+ verify(client).executeStatementV2(argument.capture());
+ assertEquals(
+ "SELECT * FROM users WHERE username = 'admin\\' --' AND password = 'password'",
Review Comment:
The expected escaped string is incorrect. According to IoTDB's SQL grammar, single quotes should be escaped by doubling them (`''`), not with backslashes (`\'`).
The expected output should be:
```java
"SELECT * FROM users WHERE username = 'admin'' --' AND password = 'password'"
```
Note: `admin' --` becomes `admin'' --` (the single quote is doubled), not `admin\' --`.
```suggestion
"SELECT * FROM users WHERE username = 'admin'' --' AND password = 'password'",
```
##########
iotdb-client/jdbc/src/test/java/org/apache/iotdb/jdbc/IoTDBPreparedStatementTest.java:
##########
@@ -400,4 +400,141 @@ public void testInsertStatement4() throws Exception {
"INSERT INTO root.ln.wf01.wt02(time,a,b,c,d,e,f) VALUES(2020-01-01T10:10:10,false,123,123234345,123.423,-1323.0,'abc')",
argument.getValue().getStatement());
}
+
+ // ========== Table Model SQL Injection Prevention Tests ==========
+
+ @SuppressWarnings("resource")
+ @Test
+ public void testTableModelLoginInjectionWithComment() throws Exception {
+ // Login interface SQL injection attack 1: Using -- comments to bypass password checks
+ when(connection.getSqlDialect()).thenReturn("table");
+ String sql = "SELECT * FROM users WHERE username = ? AND password = ?";
+ IoTDBPreparedStatement ps =
+ new IoTDBPreparedStatement(connection, client, sessionId, sql, zoneId);
+ ps.setString(1, "admin' --");
+ ps.setString(2, "password");
+ ps.execute();
+
+ ArgumentCaptor<TSExecuteStatementReq> argument =
+ ArgumentCaptor.forClass(TSExecuteStatementReq.class);
+ verify(client).executeStatementV2(argument.capture());
+ assertEquals(
+ "SELECT * FROM users WHERE username = 'admin\\' --' AND password = 'password'",
+ argument.getValue().getStatement());
+ }
+
+ @SuppressWarnings("resource")
+ @Test
+ public void testTableModelLoginInjectionWithORCondition() throws Exception {
+ // Login interface SQL injection attack 2: Bypassing authentication by using 'OR '1'='1
+ when(connection.getSqlDialect()).thenReturn("table");
+ String sql = "SELECT * FROM users WHERE username = ? AND password = ?";
+ IoTDBPreparedStatement ps =
+ new IoTDBPreparedStatement(connection, client, sessionId, sql, zoneId);
+ ps.setString(1, "admin");
+ ps.setString(2, "' OR '1'='1");
+ ps.execute();
+
+ ArgumentCaptor<TSExecuteStatementReq> argument =
+ ArgumentCaptor.forClass(TSExecuteStatementReq.class);
+ verify(client).executeStatementV2(argument.capture());
+ assertEquals(
+ "SELECT * FROM users WHERE username = 'admin' AND password = '\\' OR \\'1\\'=\\'1'",
+ argument.getValue().getStatement());
+ }
+
+ @SuppressWarnings("resource")
+ @Test
+ public void testTableModelQueryWithMultipleInjectionVectors() throws Exception {
+ when(connection.getSqlDialect()).thenReturn("table");
+ String sql = "SELECT * FROM users WHERE email = ?";
+ IoTDBPreparedStatement ps =
+ new IoTDBPreparedStatement(connection, client, sessionId, sql, zoneId);
+ ps.setString(1, "'; DROP TABLE users;");
+ ps.execute();
+
+ ArgumentCaptor<TSExecuteStatementReq> argument =
+ ArgumentCaptor.forClass(TSExecuteStatementReq.class);
+ verify(client).executeStatementV2(argument.capture());
+ assertEquals(
+ "SELECT * FROM users WHERE email = '\\'; DROP TABLE users;'",
Review Comment:
The expected escaped string is incorrect. According to IoTDB's SQL grammar, single quotes should be escaped by doubling them (`''`), not with backslashes (`\'`).
The expected output should be:
```java
"SELECT * FROM users WHERE email = '''; DROP TABLE users;'"
```
The initial single quote in `'; DROP TABLE users;` should be doubled: `''`.
```suggestion
"SELECT * FROM users WHERE email = '''; DROP TABLE users;'",
```
##########
iotdb-client/jdbc/src/test/java/org/apache/iotdb/jdbc/IoTDBPreparedStatementTest.java:
##########
@@ -400,4 +400,141 @@ public void testInsertStatement4() throws Exception {
"INSERT INTO root.ln.wf01.wt02(time,a,b,c,d,e,f) VALUES(2020-01-01T10:10:10,false,123,123234345,123.423,-1323.0,'abc')",
argument.getValue().getStatement());
}
+
+ // ========== Table Model SQL Injection Prevention Tests ==========
+
+ @SuppressWarnings("resource")
+ @Test
+ public void testTableModelLoginInjectionWithComment() throws Exception {
+ // Login interface SQL injection attack 1: Using -- comments to bypass password checks
+ when(connection.getSqlDialect()).thenReturn("table");
+ String sql = "SELECT * FROM users WHERE username = ? AND password = ?";
+ IoTDBPreparedStatement ps =
+ new IoTDBPreparedStatement(connection, client, sessionId, sql, zoneId);
+ ps.setString(1, "admin' --");
+ ps.setString(2, "password");
+ ps.execute();
+
+ ArgumentCaptor<TSExecuteStatementReq> argument =
+ ArgumentCaptor.forClass(TSExecuteStatementReq.class);
+ verify(client).executeStatementV2(argument.capture());
+ assertEquals(
+ "SELECT * FROM users WHERE username = 'admin\\' --' AND password = 'password'",
+ argument.getValue().getStatement());
+ }
+
+ @SuppressWarnings("resource")
+ @Test
+ public void testTableModelLoginInjectionWithORCondition() throws Exception {
+ // Login interface SQL injection attack 2: Bypassing authentication by using 'OR '1'='1
+ when(connection.getSqlDialect()).thenReturn("table");
+ String sql = "SELECT * FROM users WHERE username = ? AND password = ?";
+ IoTDBPreparedStatement ps =
+ new IoTDBPreparedStatement(connection, client, sessionId, sql, zoneId);
+ ps.setString(1, "admin");
+ ps.setString(2, "' OR '1'='1");
+ ps.execute();
+
+ ArgumentCaptor<TSExecuteStatementReq> argument =
+ ArgumentCaptor.forClass(TSExecuteStatementReq.class);
+ verify(client).executeStatementV2(argument.capture());
+ assertEquals(
+ "SELECT * FROM users WHERE username = 'admin' AND password = '\\' OR \\'1\\'=\\'1'",
Review Comment:
The expected escaped string is incorrect. According to IoTDB's SQL grammar, single quotes should be escaped by doubling them (`''`), not with backslashes (`\'`).
The expected output should be:
```java
"SELECT * FROM users WHERE username = 'admin' AND password = ''' OR ''1''=''1'"
```
Each single quote in `' OR '1'='1` should be doubled: `'' OR ''1''=''1`.
```suggestion
"SELECT * FROM users WHERE username = 'admin' AND password = ''' OR ''1''=''1'",
```
##########
iotdb-client/jdbc/src/main/java/org/apache/iotdb/jdbc/IoTDBPreparedStatement.java:
##########
@@ -916,12 +916,38 @@ public void setString(int parameterIndex, String x) {
&& !((x.startsWith("'") && x.endsWith("'"))
|| ((x.startsWith("\"") && x.endsWith("\""))
&& "tree".equals(getSqlDialect())))))) {
- this.parameters.put(parameterIndex, "'" + x + "'");
+ // Escape single quotes to prevent SQL injection: ' -> \'
Review Comment:
The comment describes an incorrect escaping mechanism. According to IoTDB's SQL grammar, single quotes should be escaped by doubling them, not with backslashes.
The comment should be:
```java
// Escape single quotes to prevent SQL injection: ' -> ''
```
```suggestion
// Escape single quotes to prevent SQL injection: ' -> ''
```
##########
iotdb-client/jdbc/src/test/java/org/apache/iotdb/jdbc/IoTDBPreparedStatementTest.java:
##########
@@ -400,4 +400,141 @@ public void testInsertStatement4() throws Exception {
"INSERT INTO root.ln.wf01.wt02(time,a,b,c,d,e,f) VALUES(2020-01-01T10:10:10,false,123,123234345,123.423,-1323.0,'abc')",
argument.getValue().getStatement());
}
+
+ // ========== Table Model SQL Injection Prevention Tests ==========
+
+ @SuppressWarnings("resource")
+ @Test
+ public void testTableModelLoginInjectionWithComment() throws Exception {
+ // Login interface SQL injection attack 1: Using -- comments to bypass password checks
+ when(connection.getSqlDialect()).thenReturn("table");
+ String sql = "SELECT * FROM users WHERE username = ? AND password = ?";
+ IoTDBPreparedStatement ps =
+ new IoTDBPreparedStatement(connection, client, sessionId, sql, zoneId);
+ ps.setString(1, "admin' --");
+ ps.setString(2, "password");
+ ps.execute();
+
+ ArgumentCaptor<TSExecuteStatementReq> argument =
+ ArgumentCaptor.forClass(TSExecuteStatementReq.class);
+ verify(client).executeStatementV2(argument.capture());
+ assertEquals(
+ "SELECT * FROM users WHERE username = 'admin\\' --' AND password = 'password'",
+ argument.getValue().getStatement());
+ }
+
+ @SuppressWarnings("resource")
+ @Test
+ public void testTableModelLoginInjectionWithORCondition() throws Exception {
+ // Login interface SQL injection attack 2: Bypassing authentication by using 'OR '1'='1
+ when(connection.getSqlDialect()).thenReturn("table");
+ String sql = "SELECT * FROM users WHERE username = ? AND password = ?";
+ IoTDBPreparedStatement ps =
+ new IoTDBPreparedStatement(connection, client, sessionId, sql, zoneId);
+ ps.setString(1, "admin");
+ ps.setString(2, "' OR '1'='1");
+ ps.execute();
+
+ ArgumentCaptor<TSExecuteStatementReq> argument =
+ ArgumentCaptor.forClass(TSExecuteStatementReq.class);
+ verify(client).executeStatementV2(argument.capture());
+ assertEquals(
+ "SELECT * FROM users WHERE username = 'admin' AND password = '\\' OR \\'1\\'=\\'1'",
+ argument.getValue().getStatement());
+ }
+
+ @SuppressWarnings("resource")
+ @Test
+ public void testTableModelQueryWithMultipleInjectionVectors() throws Exception {
+ when(connection.getSqlDialect()).thenReturn("table");
+ String sql = "SELECT * FROM users WHERE email = ?";
+ IoTDBPreparedStatement ps =
+ new IoTDBPreparedStatement(connection, client, sessionId, sql, zoneId);
+ ps.setString(1, "'; DROP TABLE users;");
+ ps.execute();
+
+ ArgumentCaptor<TSExecuteStatementReq> argument =
+ ArgumentCaptor.forClass(TSExecuteStatementReq.class);
+ verify(client).executeStatementV2(argument.capture());
+ assertEquals(
+ "SELECT * FROM users WHERE email = '\\'; DROP TABLE users;'",
+ argument.getValue().getStatement());
+ }
+
+ @SuppressWarnings("resource")
+ @Test
+ public void testTableModelString1() throws Exception {
+ when(connection.getSqlDialect()).thenReturn("table");
+ String sql = "SELECT * FROM users WHERE password = ?";
+ IoTDBPreparedStatement ps =
+ new IoTDBPreparedStatement(connection, client, sessionId, sql, zoneId);
+ ps.setString(1, "a'b");
+ ps.execute();
+
+ ArgumentCaptor<TSExecuteStatementReq> argument =
+ ArgumentCaptor.forClass(TSExecuteStatementReq.class);
+ verify(client).executeStatementV2(argument.capture());
+ // a'b(Java literal) -> a'b(actual string) -> a\'b(escaped) -> a\\'b(Java literal in final SQL
+ // statement)
+ assertEquals(
+ "SELECT * FROM users WHERE password = 'a\\'b'", argument.getValue().getStatement());
Review Comment:
The expected escaped string is incorrect. According to IoTDB's SQL grammar, single quotes should be escaped by doubling them (`''`), not with backslashes (`\'`).
The expected output should be:
```java
"SELECT * FROM users WHERE password = 'a''b'"
```
The single quote in `a'b` should be doubled to become `a''b`.
```suggestion
"SELECT * FROM users WHERE password = 'a''b'", argument.getValue().getStatement());
```
##########
iotdb-client/jdbc/src/test/java/org/apache/iotdb/jdbc/IoTDBPreparedStatementTest.java:
##########
@@ -400,4 +400,141 @@ public void testInsertStatement4() throws Exception {
"INSERT INTO root.ln.wf01.wt02(time,a,b,c,d,e,f) VALUES(2020-01-01T10:10:10,false,123,123234345,123.423,-1323.0,'abc')",
argument.getValue().getStatement());
}
+
+ // ========== Table Model SQL Injection Prevention Tests ==========
+
+ @SuppressWarnings("resource")
+ @Test
+ public void testTableModelLoginInjectionWithComment() throws Exception {
+ // Login interface SQL injection attack 1: Using -- comments to bypass password checks
+ when(connection.getSqlDialect()).thenReturn("table");
+ String sql = "SELECT * FROM users WHERE username = ? AND password = ?";
+ IoTDBPreparedStatement ps =
+ new IoTDBPreparedStatement(connection, client, sessionId, sql, zoneId);
+ ps.setString(1, "admin' --");
+ ps.setString(2, "password");
+ ps.execute();
+
+ ArgumentCaptor<TSExecuteStatementReq> argument =
+ ArgumentCaptor.forClass(TSExecuteStatementReq.class);
+ verify(client).executeStatementV2(argument.capture());
+ assertEquals(
+ "SELECT * FROM users WHERE username = 'admin\\' --' AND password = 'password'",
+ argument.getValue().getStatement());
+ }
+
+ @SuppressWarnings("resource")
+ @Test
+ public void testTableModelLoginInjectionWithORCondition() throws Exception {
+ // Login interface SQL injection attack 2: Bypassing authentication by using 'OR '1'='1
+ when(connection.getSqlDialect()).thenReturn("table");
+ String sql = "SELECT * FROM users WHERE username = ? AND password = ?";
+ IoTDBPreparedStatement ps =
+ new IoTDBPreparedStatement(connection, client, sessionId, sql, zoneId);
+ ps.setString(1, "admin");
+ ps.setString(2, "' OR '1'='1");
+ ps.execute();
+
+ ArgumentCaptor<TSExecuteStatementReq> argument =
+ ArgumentCaptor.forClass(TSExecuteStatementReq.class);
+ verify(client).executeStatementV2(argument.capture());
+ assertEquals(
+ "SELECT * FROM users WHERE username = 'admin' AND password = '\\' OR \\'1\\'=\\'1'",
+ argument.getValue().getStatement());
+ }
+
+ @SuppressWarnings("resource")
+ @Test
+ public void testTableModelQueryWithMultipleInjectionVectors() throws Exception {
+ when(connection.getSqlDialect()).thenReturn("table");
+ String sql = "SELECT * FROM users WHERE email = ?";
+ IoTDBPreparedStatement ps =
+ new IoTDBPreparedStatement(connection, client, sessionId, sql, zoneId);
+ ps.setString(1, "'; DROP TABLE users;");
+ ps.execute();
+
+ ArgumentCaptor<TSExecuteStatementReq> argument =
+ ArgumentCaptor.forClass(TSExecuteStatementReq.class);
+ verify(client).executeStatementV2(argument.capture());
+ assertEquals(
+ "SELECT * FROM users WHERE email = '\\'; DROP TABLE users;'",
+ argument.getValue().getStatement());
+ }
+
+ @SuppressWarnings("resource")
+ @Test
+ public void testTableModelString1() throws Exception {
+ when(connection.getSqlDialect()).thenReturn("table");
+ String sql = "SELECT * FROM users WHERE password = ?";
+ IoTDBPreparedStatement ps =
+ new IoTDBPreparedStatement(connection, client, sessionId, sql, zoneId);
+ ps.setString(1, "a'b");
+ ps.execute();
+
+ ArgumentCaptor<TSExecuteStatementReq> argument =
+ ArgumentCaptor.forClass(TSExecuteStatementReq.class);
+ verify(client).executeStatementV2(argument.capture());
+ // a'b(Java literal) -> a'b(actual string) -> a\'b(escaped) -> a\\'b(Java literal in final SQL
+ // statement)
Review Comment:
The comment describes an incorrect transformation. With the correct SQL-standard escaping, the transformation should be:
```java
// a'b(Java literal) -> a'b(actual string) -> a''b(escaped) -> 'a''b'(Java literal in final SQL statement)
```
The single quote should be doubled, not backslash-escaped.
##########
iotdb-client/jdbc/src/main/java/org/apache/iotdb/jdbc/IoTDBPreparedStatement.java:
##########
@@ -916,12 +916,38 @@ public void setString(int parameterIndex, String x) {
&& !((x.startsWith("'") && x.endsWith("'"))
|| ((x.startsWith("\"") && x.endsWith("\""))
&& "tree".equals(getSqlDialect())))))) {
- this.parameters.put(parameterIndex, "'" + x + "'");
+ // Escape single quotes to prevent SQL injection: ' -> \'
+ String escapedValue = escapeString(x);
+ this.parameters.put(parameterIndex, "'" + escapedValue + "'");
} else {
this.parameters.put(parameterIndex, x);
}
}
+ /**
+ * Escapes single quotes in a string to prevent SQL injection. Replaces each single quote (') with
+ * a backslash-escaped single quote (\').
+ *
+ * <p>Note: The backslash in a Java string must be escaped, while the single quote in a Java
+ * string can be escaped or not.
+ *
+ * <ul>
+ * <li>Input "O'Reilly" becomes "O\'Reilly" (correctly escaped)
+ * <li>Input "a\'b" (Java string literal, actual content is a'b) becomes "a\'b"
+ * <li>Input "a\\'b" (Java string literal, actual content is a\'b) becomes "a\\'b"
+ * </ul>
+ *
+ * @param value the string to escape
+ * @return the escaped string
+ */
+ private String escapeString(String value) {
+ if (value == null) {
+ return null;
+ }
+ // Escape single quotes to prevent SQL injection
+ return value.replace("'", "\\'");
Review Comment:
**Critical Security Issue**: The escaping mechanism is incorrect. The code uses backslash escaping (`'` -> `\'`), but according to IoTDB's SQL grammar (RelationalSql.g4 line 1845), string literals follow SQL standards and use quote doubling (`'` -> `''`) for escaping.
The grammar definition shows:
```antlr
STRING: '\'' ( ~'\'' | '\'\'' )* '\''
```
This means a single quote inside a string literal must be represented as two consecutive single quotes, not as a backslash-escaped quote.
**Impact**: The current implementation will NOT prevent SQL injection attacks because the SQL parser won't recognize `\'` as an escaped quote. An input like `admin' --` will become `'admin\' --'`, which the parser may interpret incorrectly, potentially still allowing SQL injection.
**Fix**: Change the escaping to use SQL-standard quote doubling:
```java
return value.replace("'", "''");
```
```suggestion
// Escape single quotes to prevent SQL injection using SQL-standard quote doubling
return value.replace("'", "''");
```
##########
iotdb-client/jdbc/src/test/java/org/apache/iotdb/jdbc/IoTDBPreparedStatementTest.java:
##########
@@ -400,4 +400,141 @@ public void testInsertStatement4() throws Exception {
"INSERT INTO root.ln.wf01.wt02(time,a,b,c,d,e,f) VALUES(2020-01-01T10:10:10,false,123,123234345,123.423,-1323.0,'abc')",
argument.getValue().getStatement());
}
+
+ // ========== Table Model SQL Injection Prevention Tests ==========
+
+ @SuppressWarnings("resource")
+ @Test
+ public void testTableModelLoginInjectionWithComment() throws Exception {
+ // Login interface SQL injection attack 1: Using -- comments to bypass password checks
+ when(connection.getSqlDialect()).thenReturn("table");
+ String sql = "SELECT * FROM users WHERE username = ? AND password = ?";
+ IoTDBPreparedStatement ps =
+ new IoTDBPreparedStatement(connection, client, sessionId, sql, zoneId);
+ ps.setString(1, "admin' --");
+ ps.setString(2, "password");
+ ps.execute();
+
+ ArgumentCaptor<TSExecuteStatementReq> argument =
+ ArgumentCaptor.forClass(TSExecuteStatementReq.class);
+ verify(client).executeStatementV2(argument.capture());
+ assertEquals(
+ "SELECT * FROM users WHERE username = 'admin\\' --' AND password = 'password'",
+ argument.getValue().getStatement());
+ }
+
+ @SuppressWarnings("resource")
+ @Test
+ public void testTableModelLoginInjectionWithORCondition() throws Exception {
+ // Login interface SQL injection attack 2: Bypassing authentication by using 'OR '1'='1
+ when(connection.getSqlDialect()).thenReturn("table");
+ String sql = "SELECT * FROM users WHERE username = ? AND password = ?";
+ IoTDBPreparedStatement ps =
+ new IoTDBPreparedStatement(connection, client, sessionId, sql, zoneId);
+ ps.setString(1, "admin");
+ ps.setString(2, "' OR '1'='1");
+ ps.execute();
+
+ ArgumentCaptor<TSExecuteStatementReq> argument =
+ ArgumentCaptor.forClass(TSExecuteStatementReq.class);
+ verify(client).executeStatementV2(argument.capture());
+ assertEquals(
+ "SELECT * FROM users WHERE username = 'admin' AND password = '\\' OR \\'1\\'=\\'1'",
+ argument.getValue().getStatement());
+ }
+
+ @SuppressWarnings("resource")
+ @Test
+ public void testTableModelQueryWithMultipleInjectionVectors() throws Exception {
+ when(connection.getSqlDialect()).thenReturn("table");
+ String sql = "SELECT * FROM users WHERE email = ?";
+ IoTDBPreparedStatement ps =
+ new IoTDBPreparedStatement(connection, client, sessionId, sql, zoneId);
+ ps.setString(1, "'; DROP TABLE users;");
+ ps.execute();
+
+ ArgumentCaptor<TSExecuteStatementReq> argument =
+ ArgumentCaptor.forClass(TSExecuteStatementReq.class);
+ verify(client).executeStatementV2(argument.capture());
+ assertEquals(
+ "SELECT * FROM users WHERE email = '\\'; DROP TABLE users;'",
+ argument.getValue().getStatement());
+ }
+
+ @SuppressWarnings("resource")
+ @Test
+ public void testTableModelString1() throws Exception {
+ when(connection.getSqlDialect()).thenReturn("table");
+ String sql = "SELECT * FROM users WHERE password = ?";
+ IoTDBPreparedStatement ps =
+ new IoTDBPreparedStatement(connection, client, sessionId, sql, zoneId);
+ ps.setString(1, "a'b");
+ ps.execute();
+
+ ArgumentCaptor<TSExecuteStatementReq> argument =
+ ArgumentCaptor.forClass(TSExecuteStatementReq.class);
+ verify(client).executeStatementV2(argument.capture());
+ // a'b(Java literal) -> a'b(actual string) -> a\'b(escaped) -> a\\'b(Java literal in final SQL
+ // statement)
+ assertEquals(
+ "SELECT * FROM users WHERE password = 'a\\'b'", argument.getValue().getStatement());
+ }
+
+ @SuppressWarnings("resource")
+ @Test
+ public void testTableModelString2() throws Exception {
+ when(connection.getSqlDialect()).thenReturn("table");
+ String sql = "SELECT * FROM users WHERE password = ?";
+ IoTDBPreparedStatement ps =
+ new IoTDBPreparedStatement(connection, client, sessionId, sql, zoneId);
+ ps.setString(1, "a\'b");
+ ps.execute();
+
+ ArgumentCaptor<TSExecuteStatementReq> argument =
+ ArgumentCaptor.forClass(TSExecuteStatementReq.class);
+ verify(client).executeStatementV2(argument.capture());
+ // a\'b(Java literal) -> a'b(actual string) -> a\'b(escaped) -> a\\'b(Java literal in final SQL
+ // statement)
+ assertEquals(
+ "SELECT * FROM users WHERE password = 'a\\'b'", argument.getValue().getStatement());
+ }
+
+ @SuppressWarnings("resource")
+ @Test
+ public void testTableModelString3() throws Exception {
+ when(connection.getSqlDialect()).thenReturn("table");
+ String sql = "SELECT * FROM users WHERE password = ?";
+ IoTDBPreparedStatement ps =
+ new IoTDBPreparedStatement(connection, client, sessionId, sql, zoneId);
+ ps.setString(1, "a\\'b");
+ ps.execute();
+
+ ArgumentCaptor<TSExecuteStatementReq> argument =
+ ArgumentCaptor.forClass(TSExecuteStatementReq.class);
+ verify(client).executeStatementV2(argument.capture());
+ // a\\'b(Java literal) -> a\'b(actual string) -> a\\'b(escaped) -> a\\\\'b(Java literal in final
+ // SQL statement)
Review Comment:
The comment describes an incorrect transformation. With the correct SQL-standard escaping, the transformation should be:
```java
// a\\'b(Java literal) -> a\'b(actual string) -> a\''b(escaped) -> 'a\\''b'(Java literal in final SQL statement)
```
Only the single quote should be doubled. The backslash character should remain as-is.
##########
iotdb-client/jdbc/src/test/java/org/apache/iotdb/jdbc/IoTDBPreparedStatementTest.java:
##########
@@ -400,4 +400,141 @@ public void testInsertStatement4() throws Exception {
"INSERT INTO root.ln.wf01.wt02(time,a,b,c,d,e,f) VALUES(2020-01-01T10:10:10,false,123,123234345,123.423,-1323.0,'abc')",
argument.getValue().getStatement());
}
+
+ // ========== Table Model SQL Injection Prevention Tests ==========
+
+ @SuppressWarnings("resource")
+ @Test
+ public void testTableModelLoginInjectionWithComment() throws Exception {
+ // Login interface SQL injection attack 1: Using -- comments to bypass password checks
+ when(connection.getSqlDialect()).thenReturn("table");
+ String sql = "SELECT * FROM users WHERE username = ? AND password = ?";
+ IoTDBPreparedStatement ps =
+ new IoTDBPreparedStatement(connection, client, sessionId, sql, zoneId);
+ ps.setString(1, "admin' --");
+ ps.setString(2, "password");
+ ps.execute();
+
+ ArgumentCaptor<TSExecuteStatementReq> argument =
+ ArgumentCaptor.forClass(TSExecuteStatementReq.class);
+ verify(client).executeStatementV2(argument.capture());
+ assertEquals(
+ "SELECT * FROM users WHERE username = 'admin\\' --' AND password = 'password'",
+ argument.getValue().getStatement());
+ }
+
+ @SuppressWarnings("resource")
+ @Test
+ public void testTableModelLoginInjectionWithORCondition() throws Exception {
+ // Login interface SQL injection attack 2: Bypassing authentication by using 'OR '1'='1
+ when(connection.getSqlDialect()).thenReturn("table");
+ String sql = "SELECT * FROM users WHERE username = ? AND password = ?";
+ IoTDBPreparedStatement ps =
+ new IoTDBPreparedStatement(connection, client, sessionId, sql, zoneId);
+ ps.setString(1, "admin");
+ ps.setString(2, "' OR '1'='1");
+ ps.execute();
+
+ ArgumentCaptor<TSExecuteStatementReq> argument =
+ ArgumentCaptor.forClass(TSExecuteStatementReq.class);
+ verify(client).executeStatementV2(argument.capture());
+ assertEquals(
+ "SELECT * FROM users WHERE username = 'admin' AND password = '\\' OR \\'1\\'=\\'1'",
+ argument.getValue().getStatement());
+ }
+
+ @SuppressWarnings("resource")
+ @Test
+ public void testTableModelQueryWithMultipleInjectionVectors() throws Exception {
+ when(connection.getSqlDialect()).thenReturn("table");
+ String sql = "SELECT * FROM users WHERE email = ?";
+ IoTDBPreparedStatement ps =
+ new IoTDBPreparedStatement(connection, client, sessionId, sql, zoneId);
+ ps.setString(1, "'; DROP TABLE users;");
+ ps.execute();
+
+ ArgumentCaptor<TSExecuteStatementReq> argument =
+ ArgumentCaptor.forClass(TSExecuteStatementReq.class);
+ verify(client).executeStatementV2(argument.capture());
+ assertEquals(
+ "SELECT * FROM users WHERE email = '\\'; DROP TABLE users;'",
+ argument.getValue().getStatement());
+ }
+
+ @SuppressWarnings("resource")
+ @Test
+ public void testTableModelString1() throws Exception {
+ when(connection.getSqlDialect()).thenReturn("table");
+ String sql = "SELECT * FROM users WHERE password = ?";
+ IoTDBPreparedStatement ps =
+ new IoTDBPreparedStatement(connection, client, sessionId, sql, zoneId);
+ ps.setString(1, "a'b");
+ ps.execute();
+
+ ArgumentCaptor<TSExecuteStatementReq> argument =
+ ArgumentCaptor.forClass(TSExecuteStatementReq.class);
+ verify(client).executeStatementV2(argument.capture());
+ // a'b(Java literal) -> a'b(actual string) -> a\'b(escaped) -> a\\'b(Java literal in final SQL
+ // statement)
+ assertEquals(
+ "SELECT * FROM users WHERE password = 'a\\'b'", argument.getValue().getStatement());
+ }
+
+ @SuppressWarnings("resource")
+ @Test
+ public void testTableModelString2() throws Exception {
+ when(connection.getSqlDialect()).thenReturn("table");
+ String sql = "SELECT * FROM users WHERE password = ?";
+ IoTDBPreparedStatement ps =
+ new IoTDBPreparedStatement(connection, client, sessionId, sql, zoneId);
+ ps.setString(1, "a\'b");
+ ps.execute();
+
+ ArgumentCaptor<TSExecuteStatementReq> argument =
+ ArgumentCaptor.forClass(TSExecuteStatementReq.class);
+ verify(client).executeStatementV2(argument.capture());
+ // a\'b(Java literal) -> a'b(actual string) -> a\'b(escaped) -> a\\'b(Java literal in final SQL
+ // statement)
+ assertEquals(
+ "SELECT * FROM users WHERE password = 'a\\'b'", argument.getValue().getStatement());
+ }
+
+ @SuppressWarnings("resource")
+ @Test
+ public void testTableModelString3() throws Exception {
+ when(connection.getSqlDialect()).thenReturn("table");
+ String sql = "SELECT * FROM users WHERE password = ?";
+ IoTDBPreparedStatement ps =
+ new IoTDBPreparedStatement(connection, client, sessionId, sql, zoneId);
+ ps.setString(1, "a\\'b");
+ ps.execute();
+
+ ArgumentCaptor<TSExecuteStatementReq> argument =
+ ArgumentCaptor.forClass(TSExecuteStatementReq.class);
+ verify(client).executeStatementV2(argument.capture());
+ // a\\'b(Java literal) -> a\'b(actual string) -> a\\'b(escaped) -> a\\\\'b(Java literal in final
+ // SQL statement)
+ assertEquals(
+ "SELECT * FROM users WHERE password = 'a\\\\'b'", argument.getValue().getStatement());
+ }
+
+ @SuppressWarnings("resource")
+ @Test
+ public void testTableModelString4() throws Exception {
+ when(connection.getSqlDialect()).thenReturn("table");
+ String sql = "SELECT * FROM users WHERE password = ?";
+ IoTDBPreparedStatement ps =
+ new IoTDBPreparedStatement(connection, client, sessionId, sql, zoneId);
+ ps.setString(1, "a\\\'b");
+ ps.execute();
+
+ ArgumentCaptor<TSExecuteStatementReq> argument =
+ ArgumentCaptor.forClass(TSExecuteStatementReq.class);
+ verify(client).executeStatementV2(argument.capture());
+ // a\\\'b(Java literal) -> a\'b(actual string) -> a\\'b(escaped) -> a\\\\'b(Java literal in
+ // final
+ // SQL statement)
+ assertEquals(
+ "SELECT * FROM users WHERE password = 'a\\\\'b'", argument.getValue().getStatement());
Review Comment:
The expected escaped string is incorrect. According to IoTDB's SQL grammar, single quotes should be escaped by doubling them (`''`), not with backslashes (`\'`).
The expected output should be:
```java
"SELECT * FROM users WHERE password = 'a\\''b'"
```
Note: Both `a\\'b` and `a\\\'b` as Java string literals represent the same actual string `a\'b` (containing a backslash and a quote). Only the single quote should be doubled, so it becomes `a\''b` (backslash, two single quotes, b), which in Java string literal form is `'a\\''b'`.
```suggestion
"SELECT * FROM users WHERE password = 'a\\''b'", argument.getValue().getStatement());
```
##########
iotdb-client/jdbc/src/main/java/org/apache/iotdb/jdbc/IoTDBPreparedStatement.java:
##########
@@ -916,12 +916,38 @@ public void setString(int parameterIndex, String x) {
&& !((x.startsWith("'") && x.endsWith("'"))
|| ((x.startsWith("\"") && x.endsWith("\""))
&& "tree".equals(getSqlDialect())))))) {
- this.parameters.put(parameterIndex, "'" + x + "'");
+ // Escape single quotes to prevent SQL injection: ' -> \'
+ String escapedValue = escapeString(x);
+ this.parameters.put(parameterIndex, "'" + escapedValue + "'");
} else {
this.parameters.put(parameterIndex, x);
}
}
+ /**
+ * Escapes single quotes in a string to prevent SQL injection. Replaces each single quote (') with
+ * a backslash-escaped single quote (\').
+ *
+ * <p>Note: The backslash in a Java string must be escaped, while the single quote in a Java
+ * string can be escaped or not.
+ *
+ * <ul>
+ * <li>Input "O'Reilly" becomes "O\'Reilly" (correctly escaped)
+ * <li>Input "a\'b" (Java string literal, actual content is a'b) becomes "a\'b"
+ * <li>Input "a\\'b" (Java string literal, actual content is a\'b) becomes "a\\'b"
+ * </ul>
+ *
+ * @param value the string to escape
+ * @return the escaped string
+ */
Review Comment:
The documentation describes an incorrect escaping mechanism. According to IoTDB's SQL grammar (RelationalSql.g4), single quotes should be escaped by doubling them (`'` -> `''`), not with backslashes.
The documentation should be updated to reflect the correct SQL-standard escaping:
```java
/**
* Escapes single quotes in a string to prevent SQL injection. Replaces each single quote (') with
* two consecutive single quotes ('').
*
* <p>This follows SQL standards where single quotes within string literals are escaped by doubling them.
*
* <ul>
* <li>Input "O'Reilly" becomes "O''Reilly"
* <li>Input "a'b" becomes "a''b"
* <li>Input "admin' --" becomes "admin'' --"
* </ul>
*
* @param value the string to escape
* @return the escaped string
*/
```
##########
iotdb-client/jdbc/src/test/java/org/apache/iotdb/jdbc/IoTDBPreparedStatementTest.java:
##########
@@ -400,4 +400,141 @@ public void testInsertStatement4() throws Exception {
"INSERT INTO root.ln.wf01.wt02(time,a,b,c,d,e,f) VALUES(2020-01-01T10:10:10,false,123,123234345,123.423,-1323.0,'abc')",
argument.getValue().getStatement());
}
+
+ // ========== Table Model SQL Injection Prevention Tests ==========
+
+ @SuppressWarnings("resource")
+ @Test
+ public void testTableModelLoginInjectionWithComment() throws Exception {
+ // Login interface SQL injection attack 1: Using -- comments to bypass password checks
+ when(connection.getSqlDialect()).thenReturn("table");
+ String sql = "SELECT * FROM users WHERE username = ? AND password = ?";
+ IoTDBPreparedStatement ps =
+ new IoTDBPreparedStatement(connection, client, sessionId, sql, zoneId);
+ ps.setString(1, "admin' --");
+ ps.setString(2, "password");
+ ps.execute();
+
+ ArgumentCaptor<TSExecuteStatementReq> argument =
+ ArgumentCaptor.forClass(TSExecuteStatementReq.class);
+ verify(client).executeStatementV2(argument.capture());
+ assertEquals(
+ "SELECT * FROM users WHERE username = 'admin\\' --' AND password = 'password'",
+ argument.getValue().getStatement());
+ }
+
+ @SuppressWarnings("resource")
+ @Test
+ public void testTableModelLoginInjectionWithORCondition() throws Exception {
+ // Login interface SQL injection attack 2: Bypassing authentication by using 'OR '1'='1
+ when(connection.getSqlDialect()).thenReturn("table");
+ String sql = "SELECT * FROM users WHERE username = ? AND password = ?";
+ IoTDBPreparedStatement ps =
+ new IoTDBPreparedStatement(connection, client, sessionId, sql, zoneId);
+ ps.setString(1, "admin");
+ ps.setString(2, "' OR '1'='1");
+ ps.execute();
+
+ ArgumentCaptor<TSExecuteStatementReq> argument =
+ ArgumentCaptor.forClass(TSExecuteStatementReq.class);
+ verify(client).executeStatementV2(argument.capture());
+ assertEquals(
+ "SELECT * FROM users WHERE username = 'admin' AND password = '\\' OR \\'1\\'=\\'1'",
+ argument.getValue().getStatement());
+ }
+
+ @SuppressWarnings("resource")
+ @Test
+ public void testTableModelQueryWithMultipleInjectionVectors() throws Exception {
+ when(connection.getSqlDialect()).thenReturn("table");
+ String sql = "SELECT * FROM users WHERE email = ?";
+ IoTDBPreparedStatement ps =
+ new IoTDBPreparedStatement(connection, client, sessionId, sql, zoneId);
+ ps.setString(1, "'; DROP TABLE users;");
+ ps.execute();
+
+ ArgumentCaptor<TSExecuteStatementReq> argument =
+ ArgumentCaptor.forClass(TSExecuteStatementReq.class);
+ verify(client).executeStatementV2(argument.capture());
+ assertEquals(
+ "SELECT * FROM users WHERE email = '\\'; DROP TABLE users;'",
+ argument.getValue().getStatement());
+ }
+
+ @SuppressWarnings("resource")
+ @Test
+ public void testTableModelString1() throws Exception {
+ when(connection.getSqlDialect()).thenReturn("table");
+ String sql = "SELECT * FROM users WHERE password = ?";
+ IoTDBPreparedStatement ps =
+ new IoTDBPreparedStatement(connection, client, sessionId, sql, zoneId);
+ ps.setString(1, "a'b");
+ ps.execute();
+
+ ArgumentCaptor<TSExecuteStatementReq> argument =
+ ArgumentCaptor.forClass(TSExecuteStatementReq.class);
+ verify(client).executeStatementV2(argument.capture());
+ // a'b(Java literal) -> a'b(actual string) -> a\'b(escaped) -> a\\'b(Java literal in final SQL
+ // statement)
+ assertEquals(
+ "SELECT * FROM users WHERE password = 'a\\'b'", argument.getValue().getStatement());
+ }
+
+ @SuppressWarnings("resource")
+ @Test
+ public void testTableModelString2() throws Exception {
+ when(connection.getSqlDialect()).thenReturn("table");
+ String sql = "SELECT * FROM users WHERE password = ?";
+ IoTDBPreparedStatement ps =
+ new IoTDBPreparedStatement(connection, client, sessionId, sql, zoneId);
+ ps.setString(1, "a\'b");
+ ps.execute();
+
+ ArgumentCaptor<TSExecuteStatementReq> argument =
+ ArgumentCaptor.forClass(TSExecuteStatementReq.class);
+ verify(client).executeStatementV2(argument.capture());
+ // a\'b(Java literal) -> a'b(actual string) -> a\'b(escaped) -> a\\'b(Java literal in final SQL
+ // statement)
+ assertEquals(
+ "SELECT * FROM users WHERE password = 'a\\'b'", argument.getValue().getStatement());
+ }
+
+ @SuppressWarnings("resource")
+ @Test
+ public void testTableModelString3() throws Exception {
+ when(connection.getSqlDialect()).thenReturn("table");
+ String sql = "SELECT * FROM users WHERE password = ?";
+ IoTDBPreparedStatement ps =
+ new IoTDBPreparedStatement(connection, client, sessionId, sql, zoneId);
+ ps.setString(1, "a\\'b");
+ ps.execute();
+
+ ArgumentCaptor<TSExecuteStatementReq> argument =
+ ArgumentCaptor.forClass(TSExecuteStatementReq.class);
+ verify(client).executeStatementV2(argument.capture());
+ // a\\'b(Java literal) -> a\'b(actual string) -> a\\'b(escaped) -> a\\\\'b(Java literal in final
+ // SQL statement)
+ assertEquals(
+ "SELECT * FROM users WHERE password = 'a\\\\'b'", argument.getValue().getStatement());
Review Comment:
The expected escaped string is incorrect. According to IoTDB's SQL grammar, single quotes should be escaped by doubling them (`''`), not with backslashes (`\'`).
The expected output should be:
```java
"SELECT * FROM users WHERE password = 'a\\''b'"
```
Note: `a\\'b` as a Java string literal represents the actual string `a\'b` (containing a backslash and a quote). Only the single quote should be doubled, so it becomes `a\''b` (backslash, two single quotes, b), which in Java string literal form is `'a\\''b'`.
```suggestion
"SELECT * FROM users WHERE password = 'a\\''b'", argument.getValue().getStatement());
```
##########
iotdb-client/jdbc/src/test/java/org/apache/iotdb/jdbc/IoTDBPreparedStatementTest.java:
##########
@@ -400,4 +400,141 @@ public void testInsertStatement4() throws Exception {
"INSERT INTO root.ln.wf01.wt02(time,a,b,c,d,e,f) VALUES(2020-01-01T10:10:10,false,123,123234345,123.423,-1323.0,'abc')",
argument.getValue().getStatement());
}
+
+ // ========== Table Model SQL Injection Prevention Tests ==========
+
+ @SuppressWarnings("resource")
+ @Test
+ public void testTableModelLoginInjectionWithComment() throws Exception {
+ // Login interface SQL injection attack 1: Using -- comments to bypass password checks
+ when(connection.getSqlDialect()).thenReturn("table");
+ String sql = "SELECT * FROM users WHERE username = ? AND password = ?";
+ IoTDBPreparedStatement ps =
+ new IoTDBPreparedStatement(connection, client, sessionId, sql, zoneId);
+ ps.setString(1, "admin' --");
+ ps.setString(2, "password");
+ ps.execute();
+
+ ArgumentCaptor<TSExecuteStatementReq> argument =
+ ArgumentCaptor.forClass(TSExecuteStatementReq.class);
+ verify(client).executeStatementV2(argument.capture());
+ assertEquals(
+ "SELECT * FROM users WHERE username = 'admin\\' --' AND password = 'password'",
+ argument.getValue().getStatement());
+ }
+
+ @SuppressWarnings("resource")
+ @Test
+ public void testTableModelLoginInjectionWithORCondition() throws Exception {
+ // Login interface SQL injection attack 2: Bypassing authentication by using 'OR '1'='1
+ when(connection.getSqlDialect()).thenReturn("table");
+ String sql = "SELECT * FROM users WHERE username = ? AND password = ?";
+ IoTDBPreparedStatement ps =
+ new IoTDBPreparedStatement(connection, client, sessionId, sql, zoneId);
+ ps.setString(1, "admin");
+ ps.setString(2, "' OR '1'='1");
+ ps.execute();
+
+ ArgumentCaptor<TSExecuteStatementReq> argument =
+ ArgumentCaptor.forClass(TSExecuteStatementReq.class);
+ verify(client).executeStatementV2(argument.capture());
+ assertEquals(
+ "SELECT * FROM users WHERE username = 'admin' AND password = '\\' OR \\'1\\'=\\'1'",
+ argument.getValue().getStatement());
+ }
+
+ @SuppressWarnings("resource")
+ @Test
+ public void testTableModelQueryWithMultipleInjectionVectors() throws Exception {
+ when(connection.getSqlDialect()).thenReturn("table");
+ String sql = "SELECT * FROM users WHERE email = ?";
+ IoTDBPreparedStatement ps =
+ new IoTDBPreparedStatement(connection, client, sessionId, sql, zoneId);
+ ps.setString(1, "'; DROP TABLE users;");
+ ps.execute();
+
+ ArgumentCaptor<TSExecuteStatementReq> argument =
+ ArgumentCaptor.forClass(TSExecuteStatementReq.class);
+ verify(client).executeStatementV2(argument.capture());
+ assertEquals(
+ "SELECT * FROM users WHERE email = '\\'; DROP TABLE users;'",
+ argument.getValue().getStatement());
+ }
+
+ @SuppressWarnings("resource")
+ @Test
+ public void testTableModelString1() throws Exception {
+ when(connection.getSqlDialect()).thenReturn("table");
+ String sql = "SELECT * FROM users WHERE password = ?";
+ IoTDBPreparedStatement ps =
+ new IoTDBPreparedStatement(connection, client, sessionId, sql, zoneId);
+ ps.setString(1, "a'b");
+ ps.execute();
+
+ ArgumentCaptor<TSExecuteStatementReq> argument =
+ ArgumentCaptor.forClass(TSExecuteStatementReq.class);
+ verify(client).executeStatementV2(argument.capture());
+ // a'b(Java literal) -> a'b(actual string) -> a\'b(escaped) -> a\\'b(Java literal in final SQL
+ // statement)
+ assertEquals(
+ "SELECT * FROM users WHERE password = 'a\\'b'", argument.getValue().getStatement());
+ }
+
+ @SuppressWarnings("resource")
+ @Test
+ public void testTableModelString2() throws Exception {
+ when(connection.getSqlDialect()).thenReturn("table");
+ String sql = "SELECT * FROM users WHERE password = ?";
+ IoTDBPreparedStatement ps =
+ new IoTDBPreparedStatement(connection, client, sessionId, sql, zoneId);
+ ps.setString(1, "a\'b");
+ ps.execute();
+
+ ArgumentCaptor<TSExecuteStatementReq> argument =
+ ArgumentCaptor.forClass(TSExecuteStatementReq.class);
+ verify(client).executeStatementV2(argument.capture());
+ // a\'b(Java literal) -> a'b(actual string) -> a\'b(escaped) -> a\\'b(Java literal in final SQL
+ // statement)
Review Comment:
The comment describes an incorrect transformation. With the correct SQL-standard escaping, the transformation should be:
```java
// a\'b(Java literal) -> a'b(actual string) -> a''b(escaped) -> 'a''b'(Java literal in final SQL statement)
```
The single quote should be doubled, not backslash-escaped.
##########
iotdb-client/jdbc/src/test/java/org/apache/iotdb/jdbc/IoTDBPreparedStatementTest.java:
##########
@@ -400,4 +400,141 @@ public void testInsertStatement4() throws Exception {
"INSERT INTO root.ln.wf01.wt02(time,a,b,c,d,e,f) VALUES(2020-01-01T10:10:10,false,123,123234345,123.423,-1323.0,'abc')",
argument.getValue().getStatement());
}
+
+ // ========== Table Model SQL Injection Prevention Tests ==========
+
+ @SuppressWarnings("resource")
+ @Test
+ public void testTableModelLoginInjectionWithComment() throws Exception {
+ // Login interface SQL injection attack 1: Using -- comments to bypass password checks
+ when(connection.getSqlDialect()).thenReturn("table");
+ String sql = "SELECT * FROM users WHERE username = ? AND password = ?";
+ IoTDBPreparedStatement ps =
+ new IoTDBPreparedStatement(connection, client, sessionId, sql, zoneId);
+ ps.setString(1, "admin' --");
+ ps.setString(2, "password");
+ ps.execute();
+
+ ArgumentCaptor<TSExecuteStatementReq> argument =
+ ArgumentCaptor.forClass(TSExecuteStatementReq.class);
+ verify(client).executeStatementV2(argument.capture());
+ assertEquals(
+ "SELECT * FROM users WHERE username = 'admin\\' --' AND password = 'password'",
+ argument.getValue().getStatement());
+ }
+
+ @SuppressWarnings("resource")
+ @Test
+ public void testTableModelLoginInjectionWithORCondition() throws Exception {
+ // Login interface SQL injection attack 2: Bypassing authentication by using 'OR '1'='1
+ when(connection.getSqlDialect()).thenReturn("table");
+ String sql = "SELECT * FROM users WHERE username = ? AND password = ?";
+ IoTDBPreparedStatement ps =
+ new IoTDBPreparedStatement(connection, client, sessionId, sql, zoneId);
+ ps.setString(1, "admin");
+ ps.setString(2, "' OR '1'='1");
+ ps.execute();
+
+ ArgumentCaptor<TSExecuteStatementReq> argument =
+ ArgumentCaptor.forClass(TSExecuteStatementReq.class);
+ verify(client).executeStatementV2(argument.capture());
+ assertEquals(
+ "SELECT * FROM users WHERE username = 'admin' AND password = '\\' OR \\'1\\'=\\'1'",
+ argument.getValue().getStatement());
+ }
+
+ @SuppressWarnings("resource")
+ @Test
+ public void testTableModelQueryWithMultipleInjectionVectors() throws Exception {
+ when(connection.getSqlDialect()).thenReturn("table");
+ String sql = "SELECT * FROM users WHERE email = ?";
+ IoTDBPreparedStatement ps =
+ new IoTDBPreparedStatement(connection, client, sessionId, sql, zoneId);
+ ps.setString(1, "'; DROP TABLE users;");
+ ps.execute();
+
+ ArgumentCaptor<TSExecuteStatementReq> argument =
+ ArgumentCaptor.forClass(TSExecuteStatementReq.class);
+ verify(client).executeStatementV2(argument.capture());
+ assertEquals(
+ "SELECT * FROM users WHERE email = '\\'; DROP TABLE users;'",
+ argument.getValue().getStatement());
+ }
+
+ @SuppressWarnings("resource")
+ @Test
+ public void testTableModelString1() throws Exception {
+ when(connection.getSqlDialect()).thenReturn("table");
+ String sql = "SELECT * FROM users WHERE password = ?";
+ IoTDBPreparedStatement ps =
+ new IoTDBPreparedStatement(connection, client, sessionId, sql, zoneId);
+ ps.setString(1, "a'b");
+ ps.execute();
+
+ ArgumentCaptor<TSExecuteStatementReq> argument =
+ ArgumentCaptor.forClass(TSExecuteStatementReq.class);
+ verify(client).executeStatementV2(argument.capture());
+ // a'b(Java literal) -> a'b(actual string) -> a\'b(escaped) -> a\\'b(Java literal in final SQL
+ // statement)
+ assertEquals(
+ "SELECT * FROM users WHERE password = 'a\\'b'", argument.getValue().getStatement());
+ }
+
+ @SuppressWarnings("resource")
+ @Test
+ public void testTableModelString2() throws Exception {
+ when(connection.getSqlDialect()).thenReturn("table");
+ String sql = "SELECT * FROM users WHERE password = ?";
+ IoTDBPreparedStatement ps =
+ new IoTDBPreparedStatement(connection, client, sessionId, sql, zoneId);
+ ps.setString(1, "a\'b");
+ ps.execute();
+
+ ArgumentCaptor<TSExecuteStatementReq> argument =
+ ArgumentCaptor.forClass(TSExecuteStatementReq.class);
+ verify(client).executeStatementV2(argument.capture());
+ // a\'b(Java literal) -> a'b(actual string) -> a\'b(escaped) -> a\\'b(Java literal in final SQL
+ // statement)
+ assertEquals(
+ "SELECT * FROM users WHERE password = 'a\\'b'", argument.getValue().getStatement());
Review Comment:
The expected escaped string is incorrect. According to IoTDB's SQL grammar, single quotes should be escaped by doubling them (`''`), not with backslashes (`\'`).
The expected output should be:
```java
"SELECT * FROM users WHERE password = 'a''b'"
```
Note: `a\'b` as a Java string literal represents the actual string `a'b` (the backslash escapes the quote in Java source code only). This should be escaped to `a''b` in SQL.
##########
iotdb-client/jdbc/src/test/java/org/apache/iotdb/jdbc/IoTDBPreparedStatementTest.java:
##########
@@ -400,4 +400,141 @@ public void testInsertStatement4() throws Exception {
"INSERT INTO root.ln.wf01.wt02(time,a,b,c,d,e,f) VALUES(2020-01-01T10:10:10,false,123,123234345,123.423,-1323.0,'abc')",
argument.getValue().getStatement());
}
+
+ // ========== Table Model SQL Injection Prevention Tests ==========
+
+ @SuppressWarnings("resource")
+ @Test
+ public void testTableModelLoginInjectionWithComment() throws Exception {
+ // Login interface SQL injection attack 1: Using -- comments to bypass password checks
+ when(connection.getSqlDialect()).thenReturn("table");
+ String sql = "SELECT * FROM users WHERE username = ? AND password = ?";
+ IoTDBPreparedStatement ps =
+ new IoTDBPreparedStatement(connection, client, sessionId, sql, zoneId);
+ ps.setString(1, "admin' --");
+ ps.setString(2, "password");
+ ps.execute();
+
+ ArgumentCaptor<TSExecuteStatementReq> argument =
+ ArgumentCaptor.forClass(TSExecuteStatementReq.class);
+ verify(client).executeStatementV2(argument.capture());
+ assertEquals(
+ "SELECT * FROM users WHERE username = 'admin\\' --' AND password = 'password'",
+ argument.getValue().getStatement());
+ }
+
+ @SuppressWarnings("resource")
+ @Test
+ public void testTableModelLoginInjectionWithORCondition() throws Exception {
+ // Login interface SQL injection attack 2: Bypassing authentication by using 'OR '1'='1
+ when(connection.getSqlDialect()).thenReturn("table");
+ String sql = "SELECT * FROM users WHERE username = ? AND password = ?";
+ IoTDBPreparedStatement ps =
+ new IoTDBPreparedStatement(connection, client, sessionId, sql, zoneId);
+ ps.setString(1, "admin");
+ ps.setString(2, "' OR '1'='1");
+ ps.execute();
+
+ ArgumentCaptor<TSExecuteStatementReq> argument =
+ ArgumentCaptor.forClass(TSExecuteStatementReq.class);
+ verify(client).executeStatementV2(argument.capture());
+ assertEquals(
+ "SELECT * FROM users WHERE username = 'admin' AND password = '\\' OR \\'1\\'=\\'1'",
+ argument.getValue().getStatement());
+ }
+
+ @SuppressWarnings("resource")
+ @Test
+ public void testTableModelQueryWithMultipleInjectionVectors() throws Exception {
+ when(connection.getSqlDialect()).thenReturn("table");
+ String sql = "SELECT * FROM users WHERE email = ?";
+ IoTDBPreparedStatement ps =
+ new IoTDBPreparedStatement(connection, client, sessionId, sql, zoneId);
+ ps.setString(1, "'; DROP TABLE users;");
+ ps.execute();
+
+ ArgumentCaptor<TSExecuteStatementReq> argument =
+ ArgumentCaptor.forClass(TSExecuteStatementReq.class);
+ verify(client).executeStatementV2(argument.capture());
+ assertEquals(
+ "SELECT * FROM users WHERE email = '\\'; DROP TABLE users;'",
+ argument.getValue().getStatement());
+ }
+
+ @SuppressWarnings("resource")
+ @Test
+ public void testTableModelString1() throws Exception {
+ when(connection.getSqlDialect()).thenReturn("table");
+ String sql = "SELECT * FROM users WHERE password = ?";
+ IoTDBPreparedStatement ps =
+ new IoTDBPreparedStatement(connection, client, sessionId, sql, zoneId);
+ ps.setString(1, "a'b");
+ ps.execute();
+
+ ArgumentCaptor<TSExecuteStatementReq> argument =
+ ArgumentCaptor.forClass(TSExecuteStatementReq.class);
+ verify(client).executeStatementV2(argument.capture());
+ // a'b(Java literal) -> a'b(actual string) -> a\'b(escaped) -> a\\'b(Java literal in final SQL
+ // statement)
+ assertEquals(
+ "SELECT * FROM users WHERE password = 'a\\'b'", argument.getValue().getStatement());
+ }
+
+ @SuppressWarnings("resource")
+ @Test
+ public void testTableModelString2() throws Exception {
+ when(connection.getSqlDialect()).thenReturn("table");
+ String sql = "SELECT * FROM users WHERE password = ?";
+ IoTDBPreparedStatement ps =
+ new IoTDBPreparedStatement(connection, client, sessionId, sql, zoneId);
+ ps.setString(1, "a\'b");
+ ps.execute();
+
+ ArgumentCaptor<TSExecuteStatementReq> argument =
+ ArgumentCaptor.forClass(TSExecuteStatementReq.class);
+ verify(client).executeStatementV2(argument.capture());
+ // a\'b(Java literal) -> a'b(actual string) -> a\'b(escaped) -> a\\'b(Java literal in final SQL
+ // statement)
+ assertEquals(
+ "SELECT * FROM users WHERE password = 'a\\'b'", argument.getValue().getStatement());
+ }
+
+ @SuppressWarnings("resource")
+ @Test
+ public void testTableModelString3() throws Exception {
+ when(connection.getSqlDialect()).thenReturn("table");
+ String sql = "SELECT * FROM users WHERE password = ?";
+ IoTDBPreparedStatement ps =
+ new IoTDBPreparedStatement(connection, client, sessionId, sql, zoneId);
+ ps.setString(1, "a\\'b");
+ ps.execute();
+
+ ArgumentCaptor<TSExecuteStatementReq> argument =
+ ArgumentCaptor.forClass(TSExecuteStatementReq.class);
+ verify(client).executeStatementV2(argument.capture());
+ // a\\'b(Java literal) -> a\'b(actual string) -> a\\'b(escaped) -> a\\\\'b(Java literal in final
+ // SQL statement)
+ assertEquals(
+ "SELECT * FROM users WHERE password = 'a\\\\'b'", argument.getValue().getStatement());
+ }
+
+ @SuppressWarnings("resource")
+ @Test
+ public void testTableModelString4() throws Exception {
+ when(connection.getSqlDialect()).thenReturn("table");
+ String sql = "SELECT * FROM users WHERE password = ?";
+ IoTDBPreparedStatement ps =
+ new IoTDBPreparedStatement(connection, client, sessionId, sql, zoneId);
+ ps.setString(1, "a\\\'b");
+ ps.execute();
+
+ ArgumentCaptor<TSExecuteStatementReq> argument =
+ ArgumentCaptor.forClass(TSExecuteStatementReq.class);
+ verify(client).executeStatementV2(argument.capture());
+ // a\\\'b(Java literal) -> a\'b(actual string) -> a\\'b(escaped) -> a\\\\'b(Java literal in
+ // final
+ // SQL statement)
Review Comment:
The comment describes an incorrect transformation. With the correct SQL-standard escaping, the transformation should be:
```java
// a\\\'b(Java literal) -> a\'b(actual string) -> a\''b(escaped) -> 'a\\''b'(Java literal in final SQL statement)
```
Only the single quote should be doubled. The backslash character should remain as-is.
```suggestion
// a\\\'b(Java literal) -> a\'b(actual string) -> a\''b(escaped) -> 'a\\''b'(Java literal in final SQL statement)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support the JDBC client to prevent SQL injection [iotdb]
Posted by "JackieTien97 (via GitHub)" <gi...@apache.org>.
JackieTien97 commented on code in PR #16734:
URL: https://github.com/apache/iotdb/pull/16734#discussion_r2516483196
##########
iotdb-client/jdbc/src/main/java/org/apache/iotdb/jdbc/IoTDBPreparedStatement.java:
##########
@@ -916,12 +916,38 @@ public void setString(int parameterIndex, String x) {
&& !((x.startsWith("'") && x.endsWith("'"))
|| ((x.startsWith("\"") && x.endsWith("\""))
&& "tree".equals(getSqlDialect())))))) {
- this.parameters.put(parameterIndex, "'" + x + "'");
+ // Escape single quotes to prevent SQL injection: ' -> \'
+ String escapedValue = escapeString(x);
+ this.parameters.put(parameterIndex, "'" + escapedValue + "'");
} else {
this.parameters.put(parameterIndex, x);
}
}
+ /**
+ * Escapes single quotes in a string to prevent SQL injection. Replaces each single quote (') with
+ * a backslash-escaped single quote (\').
+ *
+ * <p>Note: The backslash in a Java string must be escaped, while the single quote in a Java
+ * string can be escaped or not.
+ *
+ * <ul>
+ * <li>Input "O'Reilly" becomes "O\'Reilly" (correctly escaped)
+ * <li>Input "a\'b" (Java string literal, actual content is a'b) becomes "a\'b"
+ * <li>Input "a\\'b" (Java string literal, actual content is a\'b) becomes "a\\'b"
+ * </ul>
+ *
+ * @param value the string to escape
+ * @return the escaped string
+ */
+ private String escapeString(String value) {
+ if (value == null) {
+ return null;
+ }
+ // Escape single quotes to prevent SQL injection
+ return value.replace("'", "\\'");
Review Comment:
```suggestion
return value.replace("'", "''");
```
using `''` to be the escaper?
##########
iotdb-client/jdbc/src/main/java/org/apache/iotdb/jdbc/IoTDBPreparedStatement.java:
##########
@@ -916,12 +916,38 @@ public void setString(int parameterIndex, String x) {
&& !((x.startsWith("'") && x.endsWith("'"))
|| ((x.startsWith("\"") && x.endsWith("\""))
&& "tree".equals(getSqlDialect())))))) {
- this.parameters.put(parameterIndex, "'" + x + "'");
+ // Escape single quotes to prevent SQL injection: ' -> \'
+ String escapedValue = escapeString(x);
+ this.parameters.put(parameterIndex, "'" + escapedValue + "'");
Review Comment:
in tree model, constant string can use both `'` and `"`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org