Skip to content

Commit 29fdd04

Browse files
smowtonsmowton
committed
Include switch and instanceof binding in Variable.getAnAssignedValue, and test via endsInQuote
1 parent ef6ea71 commit 29fdd04

File tree

8 files changed

+66
-20
lines changed

8 files changed

+66
-20
lines changed

‎java/ql/lib/semmle/code/java/Variable.qll

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,12 @@ class Variable extends @variable, Annotatable, Element, Modifiable {
1515
/** Gets an access to this variable. */
1616
VarAccess getAnAccess() { variableBinding(result, this) }
1717

18-
/** Gets an expression on the right-hand side of an assignment to this variable. */
18+
/**
19+
* Gets an expression assigned to this variable, either appearing on the right-hand side of an
20+
* assignment or bound to it via a binding `instanceof` expression or `switch` block.
21+
*/
1922
Expr getAnAssignedValue() {
20-
exists(LocalVariableDeclExpr e | e.getVariable() = this and result = e.getInit())
23+
exists(LocalVariableDeclExpr e | e.getVariable() = this and result = e.getInitOrPatternSource())
2124
or
2225
exists(AssignExpr e | e.getDest() = this.getAnAccess() and result = e.getSource())
2326
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,3 +9,4 @@
99
| Test.java:98:47:98:60 | queryFromField | Query built by concatenation with $@, which may be untrusted. | Test.java:97:8:97:19 | categoryName | this expression |
1010
| Test.java:108:47:108:61 | querySbToString | Query built by concatenation with $@, which may be untrusted. | Test.java:104:19:104:30 | categoryName | this expression |
1111
| Test.java:118:47:118:62 | querySb2ToString | Query built by concatenation with $@, which may be untrusted. | Test.java:114:46:114:57 | categoryName | this expression |
12+
| Test.java:221:81:221:111 | ... + ... | Query built by concatenation with $@, which may be untrusted. | Test.java:221:95:221:102 | category | this expression |

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

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,13 @@ edges
1313
| Test.java:60:29:60:35 | querySb : StringBuilder | Test.java:60:29:60:46 | toString(...) : String |
1414
| Test.java:60:29:60:46 | toString(...) : String | Test.java:62:47:62:61 | querySbToString |
1515
| Test.java:183:33:183:45 | args : String[] | Test.java:209:47:209:68 | queryWithUserTableName |
16-
| Test.java:213:26:213:38 | args : String[] | Test.java:214:11:214:14 | args : String[] |
17-
| Test.java:213:26:213:38 | args : String[] | Test.java:218:14:218:17 | args : String[] |
18-
| Test.java:214:11:214:14 | args : String[] | Test.java:29:30:29:42 | args : String[] |
19-
| Test.java:218:14:218:17 | args : String[] | Test.java:183:33:183:45 | args : String[] |
16+
| Test.java:213:34:213:46 | args : String[] | Test.java:221:81:221:111 | ... + ... |
17+
| Test.java:227:26:227:38 | args : String[] | Test.java:228:11:228:14 | args : String[] |
18+
| Test.java:227:26:227:38 | args : String[] | Test.java:232:14:232:17 | args : String[] |
19+
| Test.java:227:26:227:38 | args : String[] | Test.java:233:15:233:18 | args : String[] |
20+
| Test.java:228:11:228:14 | args : String[] | Test.java:29:30:29:42 | args : String[] |
21+
| Test.java:232:14:232:17 | args : String[] | Test.java:183:33:183:45 | args : String[] |
22+
| Test.java:233:15:233:18 | args : String[] | Test.java:213:34:213:46 | args : String[] |
2023
nodes
2124
| Mongo.java:10:29:10:41 | args : String[] | semmle.label | args : String[] |
2225
| Mongo.java:17:45:17:67 | parse(...) | semmle.label | parse(...) |
@@ -35,17 +38,21 @@ nodes
3538
| Test.java:78:46:78:50 | query | semmle.label | query |
3639
| Test.java:183:33:183:45 | args : String[] | semmle.label | args : String[] |
3740
| Test.java:209:47:209:68 | queryWithUserTableName | semmle.label | queryWithUserTableName |
38-
| Test.java:213:26:213:38 | args : String[] | semmle.label | args : String[] |
39-
| Test.java:214:11:214:14 | args : String[] | semmle.label | args : String[] |
40-
| Test.java:218:14:218:17 | args : String[] | semmle.label | args : String[] |
41+
| Test.java:213:34:213:46 | args : String[] | semmle.label | args : String[] |
42+
| Test.java:221:81:221:111 | ... + ... | semmle.label | ... + ... |
43+
| Test.java:227:26:227:38 | args : String[] | semmle.label | args : String[] |
44+
| Test.java:228:11:228:14 | args : String[] | semmle.label | args : String[] |
45+
| Test.java:232:14:232:17 | args : String[] | semmle.label | args : String[] |
46+
| Test.java:233:15:233:18 | args : String[] | semmle.label | args : String[] |
4147
subpaths
4248
#select
4349
| Mongo.java:17:45:17:67 | parse(...) | Mongo.java:10:29:10:41 | args : String[] | Mongo.java:17:45:17:67 | parse(...) | This query depends on a $@. | Mongo.java:10:29:10:41 | args | user-provided value |
4450
| Mongo.java:21:49:21:52 | json | Mongo.java:10:29:10:41 | args : String[] | Mongo.java:21:49:21:52 | json | This query depends on a $@. | Mongo.java:10:29:10:41 | args | user-provided value |
45-
| Test.java:36:47:36:52 | query1 | Test.java:213:26:213:38 | args : String[] | Test.java:36:47:36:52 | query1 | This query depends on a $@. | Test.java:213:26:213:38 | args | user-provided value |
46-
| Test.java:42:57:42:62 | query2 | Test.java:213:26:213:38 | args : String[] | Test.java:42:57:42:62 | query2 | This query depends on a $@. | Test.java:213:26:213:38 | args | user-provided value |
47-
| Test.java:50:62:50:67 | query3 | Test.java:213:26:213:38 | args : String[] | Test.java:50:62:50:67 | query3 | This query depends on a $@. | Test.java:213:26:213:38 | args | user-provided value |
48-
| Test.java:62:47:62:61 | querySbToString | Test.java:213:26:213:38 | args : String[] | Test.java:62:47:62:61 | querySbToString | This query depends on a $@. | Test.java:213:26:213:38 | args | user-provided value |
49-
| Test.java:70:40:70:44 | query | Test.java:213:26:213:38 | args : String[] | Test.java:70:40:70:44 | query | This query depends on a $@. | Test.java:213:26:213:38 | args | user-provided value |
50-
| Test.java:78:46:78:50 | query | Test.java:213:26:213:38 | args : String[] | Test.java:78:46:78:50 | query | This query depends on a $@. | Test.java:213:26:213:38 | args | user-provided value |
51-
| Test.java:209:47:209:68 | queryWithUserTableName | Test.java:213:26:213:38 | args : String[] | Test.java:209:47:209:68 | queryWithUserTableName | This query depends on a $@. | Test.java:213:26:213:38 | args | user-provided value |
51+
| Test.java:36:47:36:52 | query1 | Test.java:227:26:227:38 | args : String[] | Test.java:36:47:36:52 | query1 | This query depends on a $@. | Test.java:227:26:227:38 | args | user-provided value |
52+
| Test.java:42:57:42:62 | query2 | Test.java:227:26:227:38 | args : String[] | Test.java:42:57:42:62 | query2 | This query depends on a $@. | Test.java:227:26:227:38 | args | user-provided value |
53+
| Test.java:50:62:50:67 | query3 | Test.java:227:26:227:38 | args : String[] | Test.java:50:62:50:67 | query3 | This query depends on a $@. | Test.java:227:26:227:38 | args | user-provided value |
54+
| Test.java:62:47:62:61 | querySbToString | Test.java:227:26:227:38 | args : String[] | Test.java:62:47:62:61 | querySbToString | This query depends on a $@. | Test.java:227:26:227:38 | args | user-provided value |
55+
| Test.java:70:40:70:44 | query | Test.java:227:26:227:38 | args : String[] | Test.java:70:40:70:44 | query | This query depends on a $@. | Test.java:227:26:227:38 | args | user-provided value |
56+
| Test.java:78:46:78:50 | query | Test.java:227:26:227:38 | args : String[] | Test.java:78:46:78:50 | query | This query depends on a $@. | Test.java:227:26:227:38 | args | user-provided value |
57+
| Test.java:209:47:209:68 | queryWithUserTableName | Test.java:227:26:227:38 | args : String[] | Test.java:209:47:209:68 | queryWithUserTableName | This query depends on a $@. | Test.java:227:26:227:38 | args | user-provided value |
58+
| Test.java:221:81:221:111 | ... + ... | Test.java:227:26:227:38 | args : String[] | Test.java:221:81:221:111 | ... + ... | This query depends on a $@. | Test.java:227:26:227:38 | args | user-provided value |

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,12 +210,27 @@ private static void tableNames(String[] args) throws IOException, SQLException {
210210
}
211211
}
212212

213+
private static void bindingVars(String[] args) throws IOException, SQLException {
214+
// BAD: the category might have SQL special characters in it
215+
{
216+
String category = args[1];
217+
Statement statement = connection.createStatement();
218+
String prefix = "SELECT ITEM,PRICE FROM PRODUCT WHERE ITEM_CATEGORY='";
219+
String suffix = "' ORDER BY PRICE";
220+
switch(prefix) {
221+
case String prefixAlias when prefix.length() > 10 -> statement.executeQuery(prefixAlias + category + suffix);
222+
default -> { }
223+
}
224+
}
225+
}
226+
213227
public static void main(String[] args) throws IOException, SQLException {
214228
tainted(args);
215229
unescaped();
216230
good(args);
217231
controlledStrings();
218232
tableNames(args);
233+
bindingVars(args);
219234
}
220235
}
221236

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,16 @@
11
| <clinit> | 1 | Test.java:20:2:20:9 | FloorWax |
22
| <clinit> | 1 | Test.java:20:12:20:18 | Topping |
33
| <clinit> | 1 | Test.java:20:21:20:28 | Biscuits |
4+
| bindingVars | 3 | Test.java:216:48:216:48 | 1 |
5+
| bindingVars | 5 | Test.java:218:20:218:73 | "SELECT ITEM,PRICE FROM PRODUCT WHERE ITEM_CATEGORY='" |
6+
| bindingVars | 6 | Test.java:219:20:219:37 | "' ORDER BY PRICE" |
7+
| bindingVars | 7 | Test.java:220:11:220:16 | prefix |
8+
| bindingVars | 8 | Test.java:221:34:221:39 | prefix |
9+
| bindingVars | 8 | Test.java:221:34:221:48 | length(...) |
10+
| bindingVars | 8 | Test.java:221:34:221:53 | ... > ... |
11+
| bindingVars | 8 | Test.java:221:52:221:53 | 10 |
12+
| bindingVars | 8 | Test.java:221:81:221:91 | prefixAlias |
13+
| bindingVars | 8 | Test.java:221:106:221:111 | suffix |
414
| checkIdentifier | 1 | Validation.java:7:12:7:16 | i |
515
| checkIdentifier | 1 | Validation.java:7:16:7:16 | 0 |
616
| checkIdentifier | 1 | Validation.java:7:19:7:19 | i |

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
| bindingVars | 5 | Test.java:218:20:218:73 | "SELECT ITEM,PRICE FROM PRODUCT WHERE ITEM_CATEGORY='" |
2+
| bindingVars | 7 | Test.java:220:11:220:16 | prefix |
3+
| bindingVars | 8 | Test.java:221:34:221:39 | prefix |
4+
| bindingVars | 8 | Test.java:221:81:221:91 | prefixAlias |
15
| controlledStrings | 4 | Test.java:137:26:137:79 | "SELECT ITEM,PRICE FROM PRODUCT WHERE ITEM_CATEGORY='" |
26
| controlledStrings | 12 | Test.java:145:27:145:80 | "SELECT ITEM,PRICE FROM PRODUCT WHERE ITEM_CATEGORY='" |
37
| controlledStrings | 20 | Test.java:153:35:153:88 | "SELECT ITEM,PRICE FROM PRODUCT WHERE ITEM_CATEGORY='" |

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
//semmle-extractor-options: --javac-args -cp ${testdir}/../../../../../stubs/mongodbClient:${testdir}/../../../../../stubs/springframework-5.3.8:${testdir}/../../../../../stubs/apache-hive
1+
//semmle-extractor-options: --javac-args -cp ${testdir}/../../../../../stubs/mongodbClient:${testdir}/../../../../../stubs/springframework-5.3.8:${testdir}/../../../../../stubs/apache-hive --release 21

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

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,9 +59,15 @@
5959
| Test.java:183:22:183:31 | tableNames | 23 | Test.java:206:36:208:55 | ... + ... |
6060
| Test.java:183:22:183:31 | tableNames | 24 | Test.java:207:8:207:18 | userTabName |
6161
| Test.java:183:22:183:31 | tableNames | 26 | Test.java:209:47:209:68 | queryWithUserTableName |
62-
| Test.java:213:21:213:24 | main | 1 | Test.java:214:11:214:14 | args |
63-
| Test.java:213:21:213:24 | main | 3 | Test.java:216:8:216:11 | args |
64-
| Test.java:213:21:213:24 | main | 5 | Test.java:218:14:218:17 | args |
62+
| Test.java:213:22:213:32 | bindingVars | 3 | Test.java:216:43:216:46 | args |
63+
| Test.java:213:22:213:32 | bindingVars | 3 | Test.java:216:43:216:49 | ...[...] |
64+
| Test.java:213:22:213:32 | bindingVars | 8 | Test.java:221:81:221:102 | ... + ... |
65+
| Test.java:213:22:213:32 | bindingVars | 8 | Test.java:221:81:221:111 | ... + ... |
66+
| Test.java:213:22:213:32 | bindingVars | 8 | Test.java:221:95:221:102 | category |
67+
| Test.java:227:21:227:24 | main | 1 | Test.java:228:11:228:14 | args |
68+
| Test.java:227:21:227:24 | main | 3 | Test.java:230:8:230:11 | args |
69+
| Test.java:227:21:227:24 | main | 5 | Test.java:232:14:232:17 | args |
70+
| Test.java:227:21:227:24 | main | 6 | Test.java:233:15:233:18 | args |
6571
| Validation.java:6:21:6:35 | checkIdentifier | 1 | Validation.java:7:23:7:24 | id |
6672
| Validation.java:6:21:6:35 | checkIdentifier | 2 | Validation.java:8:13:8:14 | id |
6773
| Validation.java:6:21:6:35 | checkIdentifier | 2 | Validation.java:8:13:8:24 | charAt(...) |

0 commit comments

Comments
 (0)