Skip to content

Commit f37077d

Browse files
owen-mcowen-mc
committed
Fix list of constants sanitizer for captured variables
If a mutable allowlist flows to a captured variable in a lambda, we can't be sure that it won't have a non-constant element added, so we exclude it as a list of constants sanitizer.
1 parent 50d97ff commit f37077d

File tree

2 files changed

+33
-12
lines changed

2 files changed

+33
-12
lines changed

‎java/ql/lib/semmle/code/java/dataflow/ListOfConstantsSanitizer.qll‎

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import java
77
private import semmle.code.java.controlflow.Guards
88
private import semmle.code.java.dataflow.TaintTracking
9+
private import internal.DataFlowPrivate
910

1011
/**
1112
* A comparison against a list of compile-time constants, sanitizing taint by
@@ -174,19 +175,25 @@ module Collection {
174175
/** Holds if `e` is a collection of constants. */
175176
private predicate isCollectionOfConstants(Expr e) {
176177
forex(Expr r | r = getALocalExprFlowRoot(e) |
177-
r instanceof CollectionOfConstants
178-
or
179-
// Access a static final field to get an immutable list of constants.
180-
exists(Field f | r = f.getAnAccess() |
181-
f.isStatic() and
182-
f.isFinal() and
183-
forall(Expr v | v = f.getInitializer() | v instanceof ImmutableCollectionOfConstants) and
184-
forall(Expr fieldSource | fieldSource = f.getAnAccess().(FieldWrite).getASource() |
185-
forall(Expr root | root = getALocalExprFlowRoot(fieldSource) |
186-
root instanceof ImmutableCollectionOfConstants
187-
) and
188-
noUnsafeCalls(fieldSource)
178+
(
179+
r instanceof CollectionOfConstants
180+
or
181+
// Access a static final field to get an immutable list of constants.
182+
exists(Field f | r = f.getAnAccess() |
183+
f.isStatic() and
184+
f.isFinal() and
185+
forall(Expr v | v = f.getInitializer() | v instanceof ImmutableCollectionOfConstants) and
186+
forall(Expr fieldSource | fieldSource = f.getAnAccess().(FieldWrite).getASource() |
187+
forall(Expr root | root = getALocalExprFlowRoot(fieldSource) |
188+
root instanceof ImmutableCollectionOfConstants
189+
) and
190+
noUnsafeCalls(fieldSource)
191+
)
189192
)
193+
) and
194+
(
195+
r instanceof ImmutableCollectionOfConstants or
196+
not DataFlow::localExprFlow(r, any(CapturedVariable cv).(LocalScopeVariable).getAnAccess())
190197
)
191198
) and
192199
noUnsafeCalls(e)

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
| AllowListSanitizerWithJavaUtilList.java:260:67:260:71 | query | AllowListSanitizerWithJavaUtilList.java:48:26:48:38 | args : String[] | AllowListSanitizerWithJavaUtilList.java:260:67:260:71 | query | This query depends on a $@. | AllowListSanitizerWithJavaUtilList.java:48:26:48:38 | args | user-provided value |
1616
| AllowListSanitizerWithJavaUtilList.java:269:67:269:71 | query | AllowListSanitizerWithJavaUtilList.java:48:26:48:38 | args : String[] | AllowListSanitizerWithJavaUtilList.java:269:67:269:71 | query | This query depends on a $@. | AllowListSanitizerWithJavaUtilList.java:48:26:48:38 | args | user-provided value |
1717
| AllowListSanitizerWithJavaUtilList.java:278:67:278:71 | query | AllowListSanitizerWithJavaUtilList.java:48:26:48:38 | args : String[] | AllowListSanitizerWithJavaUtilList.java:278:67:278:71 | query | This query depends on a $@. | AllowListSanitizerWithJavaUtilList.java:48:26:48:38 | args | user-provided value |
18+
| AllowListSanitizerWithJavaUtilList.java:293:67:293:71 | query | AllowListSanitizerWithJavaUtilList.java:48:26:48:38 | args : String[] | AllowListSanitizerWithJavaUtilList.java:293:67:293:71 | query | This query depends on a $@. | AllowListSanitizerWithJavaUtilList.java:48:26:48:38 | args | user-provided value |
1819
| AllowListSanitizerWithJavaUtilSet.java:87:66:87:70 | query | AllowListSanitizerWithJavaUtilSet.java:47:26:47:38 | args : String[] | AllowListSanitizerWithJavaUtilSet.java:87:66:87:70 | query | This query depends on a $@. | AllowListSanitizerWithJavaUtilSet.java:47:26:47:38 | args | user-provided value |
1920
| AllowListSanitizerWithJavaUtilSet.java:93:66:93:70 | query | AllowListSanitizerWithJavaUtilSet.java:47:26:47:38 | args : String[] | AllowListSanitizerWithJavaUtilSet.java:93:66:93:70 | query | This query depends on a $@. | AllowListSanitizerWithJavaUtilSet.java:47:26:47:38 | args | user-provided value |
2021
| AllowListSanitizerWithJavaUtilSet.java:99:66:99:70 | query | AllowListSanitizerWithJavaUtilSet.java:47:26:47:38 | args : String[] | AllowListSanitizerWithJavaUtilSet.java:99:66:99:70 | query | This query depends on a $@. | AllowListSanitizerWithJavaUtilSet.java:47:26:47:38 | args | user-provided value |
@@ -31,6 +32,7 @@
3132
| AllowListSanitizerWithJavaUtilSet.java:259:67:259:71 | query | AllowListSanitizerWithJavaUtilSet.java:47:26:47:38 | args : String[] | AllowListSanitizerWithJavaUtilSet.java:259:67:259:71 | query | This query depends on a $@. | AllowListSanitizerWithJavaUtilSet.java:47:26:47:38 | args | user-provided value |
3233
| AllowListSanitizerWithJavaUtilSet.java:268:67:268:71 | query | AllowListSanitizerWithJavaUtilSet.java:47:26:47:38 | args : String[] | AllowListSanitizerWithJavaUtilSet.java:268:67:268:71 | query | This query depends on a $@. | AllowListSanitizerWithJavaUtilSet.java:47:26:47:38 | args | user-provided value |
3334
| AllowListSanitizerWithJavaUtilSet.java:277:67:277:71 | query | AllowListSanitizerWithJavaUtilSet.java:47:26:47:38 | args : String[] | AllowListSanitizerWithJavaUtilSet.java:277:67:277:71 | query | This query depends on a $@. | AllowListSanitizerWithJavaUtilSet.java:47:26:47:38 | args | user-provided value |
35+
| AllowListSanitizerWithJavaUtilSet.java:292:67:292:71 | query | AllowListSanitizerWithJavaUtilSet.java:47:26:47:38 | args : String[] | AllowListSanitizerWithJavaUtilSet.java:292:67:292:71 | query | This query depends on a $@. | AllowListSanitizerWithJavaUtilSet.java:47:26:47:38 | args | user-provided value |
3436
| 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 |
3537
| 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 |
3638
| 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 |
@@ -46,10 +48,12 @@ edges
4648
| AllowListSanitizerWithJavaUtilList.java:48:26:48:38 | args : String[] | AllowListSanitizerWithJavaUtilList.java:51:13:51:16 | args : String[] | provenance | |
4749
| AllowListSanitizerWithJavaUtilList.java:48:26:48:38 | args : String[] | AllowListSanitizerWithJavaUtilList.java:53:25:53:28 | args : String[] | provenance | |
4850
| AllowListSanitizerWithJavaUtilList.java:48:26:48:38 | args : String[] | AllowListSanitizerWithJavaUtilList.java:54:23:54:26 | args : String[] | provenance | |
51+
| AllowListSanitizerWithJavaUtilList.java:48:26:48:38 | args : String[] | AllowListSanitizerWithJavaUtilList.java:55:14:55:17 | args : String[] | provenance | |
4952
| AllowListSanitizerWithJavaUtilList.java:50:20:50:23 | args : String[] | AllowListSanitizerWithJavaUtilList.java:58:39:58:51 | args : String[] | provenance | |
5053
| AllowListSanitizerWithJavaUtilList.java:51:13:51:16 | args : String[] | AllowListSanitizerWithJavaUtilList.java:132:32:132:44 | args : String[] | provenance | |
5154
| AllowListSanitizerWithJavaUtilList.java:53:25:53:28 | args : String[] | AllowListSanitizerWithJavaUtilList.java:122:35:122:47 | args : String[] | provenance | |
5255
| AllowListSanitizerWithJavaUtilList.java:54:23:54:26 | args : String[] | AllowListSanitizerWithJavaUtilList.java:247:42:247:54 | args : String[] | provenance | |
56+
| AllowListSanitizerWithJavaUtilList.java:55:14:55:17 | args : String[] | AllowListSanitizerWithJavaUtilList.java:283:33:283:45 | args : String[] | provenance | |
5357
| AllowListSanitizerWithJavaUtilList.java:58:39:58:51 | args : String[] | AllowListSanitizerWithJavaUtilList.java:88:66:88:70 | query | provenance | Sink:MaD:6 |
5458
| AllowListSanitizerWithJavaUtilList.java:58:39:58:51 | args : String[] | AllowListSanitizerWithJavaUtilList.java:94:66:94:70 | query | provenance | Sink:MaD:6 |
5559
| AllowListSanitizerWithJavaUtilList.java:58:39:58:51 | args : String[] | AllowListSanitizerWithJavaUtilList.java:100:66:100:70 | query | provenance | Sink:MaD:6 |
@@ -66,14 +70,17 @@ edges
6670
| AllowListSanitizerWithJavaUtilList.java:247:42:247:54 | args : String[] | AllowListSanitizerWithJavaUtilList.java:260:67:260:71 | query | provenance | Sink:MaD:6 |
6771
| AllowListSanitizerWithJavaUtilList.java:247:42:247:54 | args : String[] | AllowListSanitizerWithJavaUtilList.java:269:67:269:71 | query | provenance | Sink:MaD:6 |
6872
| AllowListSanitizerWithJavaUtilList.java:247:42:247:54 | args : String[] | AllowListSanitizerWithJavaUtilList.java:278:67:278:71 | query | provenance | Sink:MaD:6 |
73+
| AllowListSanitizerWithJavaUtilList.java:283:33:283:45 | args : String[] | AllowListSanitizerWithJavaUtilList.java:293:67:293:71 | query | provenance | Sink:MaD:6 |
6974
| AllowListSanitizerWithJavaUtilSet.java:47:26:47:38 | args : String[] | AllowListSanitizerWithJavaUtilSet.java:49:20:49:23 | args : String[] | provenance | |
7075
| AllowListSanitizerWithJavaUtilSet.java:47:26:47:38 | args : String[] | AllowListSanitizerWithJavaUtilSet.java:50:13:50:16 | args : String[] | provenance | |
7176
| AllowListSanitizerWithJavaUtilSet.java:47:26:47:38 | args : String[] | AllowListSanitizerWithJavaUtilSet.java:52:25:52:28 | args : String[] | provenance | |
7277
| AllowListSanitizerWithJavaUtilSet.java:47:26:47:38 | args : String[] | AllowListSanitizerWithJavaUtilSet.java:53:23:53:26 | args : String[] | provenance | |
78+
| AllowListSanitizerWithJavaUtilSet.java:47:26:47:38 | args : String[] | AllowListSanitizerWithJavaUtilSet.java:54:14:54:17 | args : String[] | provenance | |
7379
| AllowListSanitizerWithJavaUtilSet.java:49:20:49:23 | args : String[] | AllowListSanitizerWithJavaUtilSet.java:57:39:57:51 | args : String[] | provenance | |
7480
| AllowListSanitizerWithJavaUtilSet.java:50:13:50:16 | args : String[] | AllowListSanitizerWithJavaUtilSet.java:131:32:131:44 | args : String[] | provenance | |
7581
| AllowListSanitizerWithJavaUtilSet.java:52:25:52:28 | args : String[] | AllowListSanitizerWithJavaUtilSet.java:121:35:121:47 | args : String[] | provenance | |
7682
| AllowListSanitizerWithJavaUtilSet.java:53:23:53:26 | args : String[] | AllowListSanitizerWithJavaUtilSet.java:246:42:246:54 | args : String[] | provenance | |
83+
| AllowListSanitizerWithJavaUtilSet.java:54:14:54:17 | args : String[] | AllowListSanitizerWithJavaUtilSet.java:282:33:282:45 | args : String[] | provenance | |
7784
| AllowListSanitizerWithJavaUtilSet.java:57:39:57:51 | args : String[] | AllowListSanitizerWithJavaUtilSet.java:87:66:87:70 | query | provenance | Sink:MaD:6 |
7885
| AllowListSanitizerWithJavaUtilSet.java:57:39:57:51 | args : String[] | AllowListSanitizerWithJavaUtilSet.java:93:66:93:70 | query | provenance | Sink:MaD:6 |
7986
| AllowListSanitizerWithJavaUtilSet.java:57:39:57:51 | args : String[] | AllowListSanitizerWithJavaUtilSet.java:99:66:99:70 | query | provenance | Sink:MaD:6 |
@@ -90,6 +97,7 @@ edges
9097
| AllowListSanitizerWithJavaUtilSet.java:246:42:246:54 | args : String[] | AllowListSanitizerWithJavaUtilSet.java:259:67:259:71 | query | provenance | Sink:MaD:6 |
9198
| AllowListSanitizerWithJavaUtilSet.java:246:42:246:54 | args : String[] | AllowListSanitizerWithJavaUtilSet.java:268:67:268:71 | query | provenance | Sink:MaD:6 |
9299
| AllowListSanitizerWithJavaUtilSet.java:246:42:246:54 | args : String[] | AllowListSanitizerWithJavaUtilSet.java:277:67:277:71 | query | provenance | Sink:MaD:6 |
100+
| AllowListSanitizerWithJavaUtilSet.java:282:33:282:45 | args : String[] | AllowListSanitizerWithJavaUtilSet.java:292:67:292:71 | query | provenance | Sink:MaD:6 |
93101
| Mongo.java:10:29:10:41 | args : String[] | Mongo.java:17:56:17:66 | stringQuery : String | provenance | |
94102
| Mongo.java:10:29:10:41 | args : String[] | Mongo.java:21:49:21:52 | json | provenance | |
95103
| Mongo.java:17:56:17:66 | stringQuery : String | Mongo.java:17:45:17:67 | parse(...) | provenance | Config |
@@ -125,6 +133,7 @@ nodes
125133
| AllowListSanitizerWithJavaUtilList.java:51:13:51:16 | args : String[] | semmle.label | args : String[] |
126134
| AllowListSanitizerWithJavaUtilList.java:53:25:53:28 | args : String[] | semmle.label | args : String[] |
127135
| AllowListSanitizerWithJavaUtilList.java:54:23:54:26 | args : String[] | semmle.label | args : String[] |
136+
| AllowListSanitizerWithJavaUtilList.java:55:14:55:17 | args : String[] | semmle.label | args : String[] |
128137
| AllowListSanitizerWithJavaUtilList.java:58:39:58:51 | args : String[] | semmle.label | args : String[] |
129138
| AllowListSanitizerWithJavaUtilList.java:88:66:88:70 | query | semmle.label | query |
130139
| AllowListSanitizerWithJavaUtilList.java:94:66:94:70 | query | semmle.label | query |
@@ -145,11 +154,14 @@ nodes
145154
| AllowListSanitizerWithJavaUtilList.java:260:67:260:71 | query | semmle.label | query |
146155
| AllowListSanitizerWithJavaUtilList.java:269:67:269:71 | query | semmle.label | query |
147156
| AllowListSanitizerWithJavaUtilList.java:278:67:278:71 | query | semmle.label | query |
157+
| AllowListSanitizerWithJavaUtilList.java:283:33:283:45 | args : String[] | semmle.label | args : String[] |
158+
| AllowListSanitizerWithJavaUtilList.java:293:67:293:71 | query | semmle.label | query |
148159
| AllowListSanitizerWithJavaUtilSet.java:47:26:47:38 | args : String[] | semmle.label | args : String[] |
149160
| AllowListSanitizerWithJavaUtilSet.java:49:20:49:23 | args : String[] | semmle.label | args : String[] |
150161
| AllowListSanitizerWithJavaUtilSet.java:50:13:50:16 | args : String[] | semmle.label | args : String[] |
151162
| AllowListSanitizerWithJavaUtilSet.java:52:25:52:28 | args : String[] | semmle.label | args : String[] |
152163
| AllowListSanitizerWithJavaUtilSet.java:53:23:53:26 | args : String[] | semmle.label | args : String[] |
164+
| AllowListSanitizerWithJavaUtilSet.java:54:14:54:17 | args : String[] | semmle.label | args : String[] |
153165
| AllowListSanitizerWithJavaUtilSet.java:57:39:57:51 | args : String[] | semmle.label | args : String[] |
154166
| AllowListSanitizerWithJavaUtilSet.java:87:66:87:70 | query | semmle.label | query |
155167
| AllowListSanitizerWithJavaUtilSet.java:93:66:93:70 | query | semmle.label | query |
@@ -170,6 +182,8 @@ nodes
170182
| AllowListSanitizerWithJavaUtilSet.java:259:67:259:71 | query | semmle.label | query |
171183
| AllowListSanitizerWithJavaUtilSet.java:268:67:268:71 | query | semmle.label | query |
172184
| AllowListSanitizerWithJavaUtilSet.java:277:67:277:71 | query | semmle.label | query |
185+
| AllowListSanitizerWithJavaUtilSet.java:282:33:282:45 | args : String[] | semmle.label | args : String[] |
186+
| AllowListSanitizerWithJavaUtilSet.java:292:67:292:71 | query | semmle.label | query |
173187
| Mongo.java:10:29:10:41 | args : String[] | semmle.label | args : String[] |
174188
| Mongo.java:17:45:17:67 | parse(...) | semmle.label | parse(...) |
175189
| Mongo.java:17:56:17:66 | stringQuery : String | semmle.label | stringQuery : String |

0 commit comments

Comments
 (0)