Skip to content

Commit 6749f7a

Browse files
aschackmullaschackmull
authored
Merge pull request #1843 from lukecartey/java/add-missing-sql-apis
Java: Add missing SQL query APIs.
2 parents 394563d + e118f9a commit 6749f7a

File tree

12 files changed

+263
-142
lines changed

12 files changed

+263
-142
lines changed

‎change-notes/1.23/analysis-java.md‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
# Improvements to Java analysis
2+
3+
The following changes in version 1.23 affect Java analysis in all applications.
4+
5+
## Changes to existing queries
6+
7+
| **Query** | **Expected impact** | **Change** |
8+
|------------------------------|------------------------|-----------------------------------|
9+
| Query built from user-controlled sources (`java/sql-injection`) | More results | The query now identifies arguments to `Statement.executeLargeUpdate` and `Connection.prepareCall` as SQL expressions sinks. |
10+
| Query built from local-user-controlled sources (`java/sql-injection-local`) | More results | The query now identifies arguments to `Statement.executeLargeUpdate` and `Connection.prepareCall` as SQL expressions sinks. |
11+
| Query built without neutralizing special characters (`java/concatenated-sql-query`) | More results | The query now identifies arguments to `Statement.executeLargeUpdate` and `Connection.prepareCall` as SQL expressions sinks. |

‎java/ql/src/semmle/code/java/frameworks/Jdbc.qll‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,14 @@ class ConnectionPrepareStatement extends Method {
3434
}
3535
}
3636

37+
/** A method with the name `prepareCall` declared in `java.sql.Connection`. */
38+
class ConnectionPrepareCall extends Method {
39+
ConnectionPrepareCall() {
40+
getDeclaringType() instanceof TypeConnection and
41+
hasName("prepareCall")
42+
}
43+
}
44+
3745
/** A method with the name `executeQuery` declared in `java.sql.Statement`. */
3846
class StatementExecuteQuery extends Method {
3947
StatementExecuteQuery() {
@@ -58,6 +66,14 @@ class MethodStatementExecuteUpdate extends Method {
5866
}
5967
}
6068

69+
/** A method with the name `executeLargeUpdate` declared in `java.sql.Statement`. */
70+
class MethodStatementExecuteLargeUpdate extends Method {
71+
MethodStatementExecuteLargeUpdate() {
72+
getDeclaringType() instanceof TypeStatement and
73+
hasName("executeLargeUpdate")
74+
}
75+
}
76+
6177
/** A method with the name `addBatch` declared in `java.sql.Statement`. */
6278
class MethodStatementAddBatch extends Method {
6379
MethodStatementAddBatch() {
@@ -87,9 +103,11 @@ class SqlExpr extends Expr {
87103
method = call.getMethod() and
88104
(
89105
method instanceof ConnectionPrepareStatement or
106+
method instanceof ConnectionPrepareCall or
90107
method instanceof StatementExecuteQuery or
91108
method instanceof MethodStatementExecute or
92109
method instanceof MethodStatementExecuteUpdate or
110+
method instanceof MethodStatementExecuteLargeUpdate or
93111
method instanceof MethodStatementAddBatch
94112
)
95113
)

‎java/ql/test/query-tests/security/CWE-089/semmle/examples/SqlTaintedLocal.expected‎

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,20 @@
11
edges
22
| Test.java:29:30:29:42 | args [String[]] | Test.java:36:47:36:52 | query1 |
3-
| Test.java:29:30:29:42 | args [String[]] | Test.java:44:62:44:67 | query3 |
4-
| Test.java:29:30:29:42 | args [String[]] | Test.java:56:47:56:61 | querySbToString |
5-
| Test.java:160:33:160:45 | args [String[]] | Test.java:186:47:186:68 | queryWithUserTableName |
6-
| Test.java:190:26:190:38 | args [String[]] | Test.java:191:11:191:14 | args [String[]] |
7-
| Test.java:190:26:190:38 | args [String[]] | Test.java:195:14:195:17 | args [String[]] |
8-
| Test.java:191:11:191:14 | args [String[]] | Test.java:29:30:29:42 | args [String[]] |
9-
| Test.java:195:14:195:17 | args [String[]] | Test.java:160:33:160:45 | args [String[]] |
3+
| Test.java:29:30:29:42 | args [String[]] | Test.java:42:57:42:62 | query2 |
4+
| Test.java:29:30:29:42 | args [String[]] | Test.java:50:62:50:67 | query3 |
5+
| Test.java:29:30:29:42 | args [String[]] | Test.java:62:47:62:61 | querySbToString |
6+
| Test.java:29:30:29:42 | args [String[]] | Test.java:70:40:70:44 | query |
7+
| Test.java:29:30:29:42 | args [String[]] | Test.java:78:46:78:50 | query |
8+
| Test.java:183:33:183:45 | args [String[]] | Test.java:209:47:209:68 | queryWithUserTableName |
9+
| Test.java:213:26:213:38 | args [String[]] | Test.java:214:11:214:14 | args [String[]] |
10+
| Test.java:213:26:213:38 | args [String[]] | Test.java:218:14:218:17 | args [String[]] |
11+
| Test.java:214:11:214:14 | args [String[]] | Test.java:29:30:29:42 | args [String[]] |
12+
| Test.java:218:14:218:17 | args [String[]] | Test.java:183:33:183:45 | args [String[]] |
1013
#select
11-
| Test.java:36:47:36:52 | query1 | Test.java:190:26:190:38 | args [String[]] | Test.java:36:47:36:52 | query1 | Query might include code from $@. | Test.java:190:26:190:38 | args | this user input |
12-
| Test.java:44:62:44:67 | query3 | Test.java:190:26:190:38 | args [String[]] | Test.java:44:62:44:67 | query3 | Query might include code from $@. | Test.java:190:26:190:38 | args | this user input |
13-
| Test.java:56:47:56:61 | querySbToString | Test.java:190:26:190:38 | args [String[]] | Test.java:56:47:56:61 | querySbToString | Query might include code from $@. | Test.java:190:26:190:38 | args | this user input |
14-
| Test.java:186:47:186:68 | queryWithUserTableName | Test.java:190:26:190:38 | args [String[]] | Test.java:186:47:186:68 | queryWithUserTableName | Query might include code from $@. | Test.java:190:26:190:38 | args | this user input |
14+
| Test.java:36:47:36:52 | query1 | Test.java:213:26:213:38 | args [String[]] | Test.java:36:47:36:52 | query1 | Query might include code from $@. | Test.java:213:26:213:38 | args | this user input |
15+
| Test.java:42:57:42:62 | query2 | Test.java:213:26:213:38 | args [String[]] | Test.java:42:57:42:62 | query2 | Query might include code from $@. | Test.java:213:26:213:38 | args | this user input |
16+
| Test.java:50:62:50:67 | query3 | Test.java:213:26:213:38 | args [String[]] | Test.java:50:62:50:67 | query3 | Query might include code from $@. | Test.java:213:26:213:38 | args | this user input |
17+
| Test.java:62:47:62:61 | querySbToString | Test.java:213:26:213:38 | args [String[]] | Test.java:62:47:62:61 | querySbToString | Query might include code from $@. | Test.java:213:26:213:38 | args | this user input |
18+
| Test.java:70:40:70:44 | query | Test.java:213:26:213:38 | args [String[]] | Test.java:70:40:70:44 | query | Query might include code from $@. | Test.java:213:26:213:38 | args | this user input |
19+
| Test.java:78:46:78:50 | query | Test.java:213:26:213:38 | args [String[]] | Test.java:78:46:78:50 | query | Query might include code from $@. | Test.java:213:26:213:38 | args | this user input |
20+
| Test.java:209:47:209:68 | queryWithUserTableName | Test.java:213:26:213:38 | args [String[]] | Test.java:209:47:209:68 | queryWithUserTableName | Query might include code from $@. | Test.java:213:26:213:38 | args | this user input |

‎java/ql/test/query-tests/security/CWE-089/semmle/examples/SqlUnescaped.expected‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
| Test.java:36:47:36:52 | query1 | Query might not neutralize special characters in $@. | Test.java:35:8:35:15 | category | this expression |
2-
| Test.java:44:62:44:67 | query3 | Query might not neutralize special characters in $@. | Test.java:43:8:43:15 | category | this expression |
3-
| Test.java:56:47:56:61 | querySbToString | Query might not neutralize special characters in $@. | Test.java:52:19:52:26 | category | this expression |
4-
| Test.java:75:47:75:60 | queryFromField | Query might not neutralize special characters in $@. | Test.java:74:8:74:19 | categoryName | this expression |
5-
| Test.java:85:47:85:61 | querySbToString | Query might not neutralize special characters in $@. | Test.java:81:19:81:30 | categoryName | this expression |
6-
| Test.java:95:47:95:62 | querySb2ToString | Query might not neutralize special characters in $@. | Test.java:91:46:91:57 | categoryName | this expression |
2+
| Test.java:42:57:42:62 | query2 | Query might not neutralize special characters in $@. | Test.java:41:51:41:52 | id | this expression |
3+
| Test.java:50:62:50:67 | query3 | Query might not neutralize special characters in $@. | Test.java:49:8:49:15 | category | this expression |
4+
| Test.java:62:47:62:61 | querySbToString | Query might not neutralize special characters in $@. | Test.java:58:19:58:26 | category | this expression |
5+
| Test.java:70:40:70:44 | query | Query might not neutralize special characters in $@. | Test.java:69:50:69:54 | price | this expression |
6+
| Test.java:70:40:70:44 | query | Query might not neutralize special characters in $@. | Test.java:69:77:69:80 | item | this expression |
7+
| Test.java:78:46:78:50 | query | Query might not neutralize special characters in $@. | Test.java:77:50:77:54 | price | this expression |
8+
| Test.java:78:46:78:50 | query | Query might not neutralize special characters in $@. | Test.java:77:77:77:80 | item | this expression |
9+
| Test.java:98:47:98:60 | queryFromField | Query might not neutralize special characters in $@. | Test.java:97:8:97:19 | categoryName | this expression |
10+
| Test.java:108:47:108:61 | querySbToString | Query might not neutralize special characters in $@. | Test.java:104:19:104:30 | categoryName | this expression |
11+
| Test.java:118:47:118:62 | querySb2ToString | Query might not neutralize special characters in $@. | Test.java:114:46:114:57 | categoryName | this expression |

‎java/ql/test/query-tests/security/CWE-089/semmle/examples/Test.java‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,13 @@ private static void tainted(String[] args) throws IOException, SQLException {
3535
+ category + "' ORDER BY PRICE";
3636
ResultSet results = statement.executeQuery(query1);
3737
}
38-
38+
// BAD: don't use user input when building a prepared call
39+
{
40+
String id = args[1];
41+
String query2 = "{ call get_product_by_id('" + id + "',?,?,?) }";
42+
PreparedStatement statement = connection.prepareCall(query2);
43+
ResultSet results = statement.executeQuery();
44+
}
3945
// BAD: don't use user input when building a prepared query
4046
{
4147
String category = args[1];
@@ -55,6 +61,23 @@ private static void tainted(String[] args) throws IOException, SQLException {
5561
Statement statement = connection.createStatement();
5662
ResultSet results = statement.executeQuery(querySbToString);
5763
}
64+
// BAD: executeUpdate
65+
{
66+
String item = args[1];
67+
String price = args[2];
68+
Statement statement = connection.createStatement();
69+
String query = "UPDATE PRODUCT SET PRICE='" + price + "' WHERE ITEM='" + item + "'";
70+
int count = statement.executeUpdate(query);
71+
}
72+
// BAD: executeUpdate
73+
{
74+
String item = args[1];
75+
String price = args[2];
76+
Statement statement = connection.createStatement();
77+
String query = "UPDATE PRODUCT SET PRICE='" + price + "' WHERE ITEM='" + item + "'";
78+
long count = statement.executeLargeUpdate(query);
79+
}
80+
5881
// OK: validate the input first
5982
{
6083
String category = args[1];

‎java/ql/test/query-tests/security/CWE-089/semmle/examples/ValidatedVariable.expected‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
| Test.java:64:8:64:15 | category |
1+
| Test.java:87:8:87:15 | category |

0 commit comments

Comments
 (0)