Skip to content

Commit b18f518

Browse files
erik-krogherik-krogh
committed
regain the lost property presence result
1 parent 6fccf5a commit b18f518

4 files changed

Lines changed: 65 additions & 14 deletions

File tree

‎javascript/ql/src/semmle/javascript/security/dataflow/UnsafeJQueryPlugin.qll‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,8 @@ module UnsafeJQueryPlugin {
2929

3030
override predicate isAdditionalTaintStep(DataFlow::Node src, DataFlow::Node sink) {
3131
// jQuery plugins tend to be implemented as classes that store data in fields initialized by the constructor.
32-
DataFlow::localFieldStep(src, sink)
32+
DataFlow::localFieldStep(src, sink) or
33+
aliasPropertyPresenceStep(src, sink)
3334
}
3435

3536
override predicate isSanitizerEdge(DataFlow::Node pred, DataFlow::Node succ) {
@@ -51,4 +52,20 @@ module UnsafeJQueryPlugin {
5152
node instanceof PropertyPresenceSanitizer
5253
}
5354
}
55+
56+
/**
57+
* Holds if there is a taint-step from `src` to `sink`,
58+
* where `src` is a property read that acts as a sanitizer for the base,
59+
* and `sink` is that same property read from the same base.
60+
*
61+
* For an condition like `if(foo.bar) {...}`, the base `foo` is sanitized but the property `foo.bar` is not.
62+
* With this taint-step we regain that `foo.bar` is tainted, because `PropertyPresenceSanitizer` could remove it.
63+
*/
64+
private predicate aliasPropertyPresenceStep(DataFlow::Node src, DataFlow::Node sink) {
65+
exists(PropertyPresenceSanitizer sanitizer, DataFlow::PropRead read | read = src |
66+
read = sanitizer.getPropRead() and
67+
sink = AccessPath::getAnAliasedSourceNode(read) and
68+
read.getBasicBlock().(ReachableBasicBlock).strictlyDominates(sink.getBasicBlock())
69+
)
70+
}
5471
}

‎javascript/ql/src/semmle/javascript/security/dataflow/UnsafeJQueryPluginCustomizations.qll‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,13 @@ module UnsafeJQueryPlugin {
152152
)
153153
}
154154

155+
/**
156+
* Gets the property read that is used to sanitize the base value.
157+
*/
158+
DataFlow::PropRead getPropRead() {
159+
result = this
160+
}
161+
155162
override predicate sanitizes(boolean outcome, Expr e) {
156163
outcome = polarity and
157164
e = input.asExpr()

‎javascript/ql/test/query-tests/Security/CWE-079/UnsafeJQueryPlugin.expected‎

Lines changed: 39 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ nodes
66
| unsafe-jquery-plugin.js:5:5:5:11 | options |
77
| unsafe-jquery-plugin.js:5:5:5:18 | options.target |
88
| unsafe-jquery-plugin.js:5:5:5:18 | options.target |
9+
| unsafe-jquery-plugin.js:7:17:7:23 | options |
10+
| unsafe-jquery-plugin.js:7:17:7:30 | options.target |
911
| unsafe-jquery-plugin.js:11:7:11:29 | target |
1012
| unsafe-jquery-plugin.js:11:16:11:22 | options |
1113
| unsafe-jquery-plugin.js:11:16:11:29 | options.target |
@@ -23,6 +25,15 @@ nodes
2325
| unsafe-jquery-plugin.js:52:6:52:11 | target |
2426
| unsafe-jquery-plugin.js:60:6:60:11 | target |
2527
| unsafe-jquery-plugin.js:60:6:60:11 | target |
28+
| unsafe-jquery-plugin.js:65:47:65:53 | options |
29+
| unsafe-jquery-plugin.js:65:47:65:53 | options |
30+
| unsafe-jquery-plugin.js:67:24:67:44 | $.exten ... ptions) |
31+
| unsafe-jquery-plugin.js:67:33:67:34 | {} |
32+
| unsafe-jquery-plugin.js:67:37:67:43 | options |
33+
| unsafe-jquery-plugin.js:68:7:68:18 | this.options |
34+
| unsafe-jquery-plugin.js:68:7:68:25 | this.options.parent |
35+
| unsafe-jquery-plugin.js:68:45:68:63 | this.options.parent |
36+
| unsafe-jquery-plugin.js:68:45:68:63 | this.options.parent |
2637
| unsafe-jquery-plugin.js:71:38:71:44 | options |
2738
| unsafe-jquery-plugin.js:71:38:71:44 | options |
2839
| unsafe-jquery-plugin.js:72:5:72:11 | options |
@@ -55,9 +66,6 @@ nodes
5566
| unsafe-jquery-plugin.js:102:13:105:13 | $.exten ... ptions) |
5667
| unsafe-jquery-plugin.js:102:22:105:3 | {\\n\\t\\t\\tme ... in'\\n\\t\\t} |
5768
| unsafe-jquery-plugin.js:105:6:105:12 | options |
58-
| unsafe-jquery-plugin.js:106:5:106:11 | options |
59-
| unsafe-jquery-plugin.js:106:5:106:16 | options.menu |
60-
| unsafe-jquery-plugin.js:106:5:106:16 | options.menu |
6169
| unsafe-jquery-plugin.js:107:5:107:11 | options |
6270
| unsafe-jquery-plugin.js:107:5:107:18 | options.target |
6371
| unsafe-jquery-plugin.js:107:5:107:18 | options.target |
@@ -67,9 +75,6 @@ nodes
6775
| unsafe-jquery-plugin.js:115:13:115:58 | $.exten ... ptions) |
6876
| unsafe-jquery-plugin.js:115:22:115:23 | {} |
6977
| unsafe-jquery-plugin.js:115:51:115:57 | options |
70-
| unsafe-jquery-plugin.js:116:5:116:11 | options |
71-
| unsafe-jquery-plugin.js:116:5:116:16 | options.menu |
72-
| unsafe-jquery-plugin.js:116:5:116:16 | options.menu |
7378
| unsafe-jquery-plugin.js:117:5:117:11 | options |
7479
| unsafe-jquery-plugin.js:117:5:117:18 | options.target |
7580
| unsafe-jquery-plugin.js:117:5:117:18 | options.target |
@@ -96,6 +101,10 @@ nodes
96101
| unsafe-jquery-plugin.js:136:5:136:29 | options ... elector |
97102
| unsafe-jquery-plugin.js:153:38:153:44 | options |
98103
| unsafe-jquery-plugin.js:153:38:153:44 | options |
104+
| unsafe-jquery-plugin.js:154:16:154:22 | options |
105+
| unsafe-jquery-plugin.js:154:16:154:29 | options.target |
106+
| unsafe-jquery-plugin.js:156:3:156:9 | options |
107+
| unsafe-jquery-plugin.js:156:3:156:16 | options.target |
99108
| unsafe-jquery-plugin.js:157:44:157:50 | options |
100109
| unsafe-jquery-plugin.js:157:44:157:57 | options.target |
101110
| unsafe-jquery-plugin.js:157:44:157:59 | options.target.a |
@@ -119,10 +128,15 @@ edges
119128
| unsafe-jquery-plugin.js:2:38:2:44 | options | unsafe-jquery-plugin.js:3:5:3:11 | options |
120129
| unsafe-jquery-plugin.js:2:38:2:44 | options | unsafe-jquery-plugin.js:5:5:5:11 | options |
121130
| unsafe-jquery-plugin.js:2:38:2:44 | options | unsafe-jquery-plugin.js:5:5:5:11 | options |
131+
| unsafe-jquery-plugin.js:2:38:2:44 | options | unsafe-jquery-plugin.js:7:17:7:23 | options |
132+
| unsafe-jquery-plugin.js:2:38:2:44 | options | unsafe-jquery-plugin.js:7:17:7:23 | options |
122133
| unsafe-jquery-plugin.js:2:38:2:44 | options | unsafe-jquery-plugin.js:11:16:11:22 | options |
123134
| unsafe-jquery-plugin.js:2:38:2:44 | options | unsafe-jquery-plugin.js:11:16:11:22 | options |
124135
| unsafe-jquery-plugin.js:5:5:5:11 | options | unsafe-jquery-plugin.js:5:5:5:18 | options.target |
125136
| unsafe-jquery-plugin.js:5:5:5:11 | options | unsafe-jquery-plugin.js:5:5:5:18 | options.target |
137+
| unsafe-jquery-plugin.js:5:5:5:18 | options.target | unsafe-jquery-plugin.js:11:16:11:29 | options.target |
138+
| unsafe-jquery-plugin.js:7:17:7:23 | options | unsafe-jquery-plugin.js:7:17:7:30 | options.target |
139+
| unsafe-jquery-plugin.js:7:17:7:30 | options.target | unsafe-jquery-plugin.js:11:16:11:29 | options.target |
126140
| unsafe-jquery-plugin.js:11:7:11:29 | target | unsafe-jquery-plugin.js:22:6:22:11 | target |
127141
| unsafe-jquery-plugin.js:11:7:11:29 | target | unsafe-jquery-plugin.js:22:6:22:11 | target |
128142
| unsafe-jquery-plugin.js:11:7:11:29 | target | unsafe-jquery-plugin.js:30:6:30:11 | target |
@@ -139,6 +153,15 @@ edges
139153
| unsafe-jquery-plugin.js:11:7:11:29 | target | unsafe-jquery-plugin.js:60:6:60:11 | target |
140154
| unsafe-jquery-plugin.js:11:16:11:22 | options | unsafe-jquery-plugin.js:11:16:11:29 | options.target |
141155
| unsafe-jquery-plugin.js:11:16:11:29 | options.target | unsafe-jquery-plugin.js:11:7:11:29 | target |
156+
| unsafe-jquery-plugin.js:65:47:65:53 | options | unsafe-jquery-plugin.js:67:37:67:43 | options |
157+
| unsafe-jquery-plugin.js:65:47:65:53 | options | unsafe-jquery-plugin.js:67:37:67:43 | options |
158+
| unsafe-jquery-plugin.js:67:24:67:44 | $.exten ... ptions) | unsafe-jquery-plugin.js:68:7:68:18 | this.options |
159+
| unsafe-jquery-plugin.js:67:33:67:34 | {} | unsafe-jquery-plugin.js:67:24:67:44 | $.exten ... ptions) |
160+
| unsafe-jquery-plugin.js:67:37:67:43 | options | unsafe-jquery-plugin.js:67:24:67:44 | $.exten ... ptions) |
161+
| unsafe-jquery-plugin.js:67:37:67:43 | options | unsafe-jquery-plugin.js:67:33:67:34 | {} |
162+
| unsafe-jquery-plugin.js:68:7:68:18 | this.options | unsafe-jquery-plugin.js:68:7:68:25 | this.options.parent |
163+
| unsafe-jquery-plugin.js:68:7:68:25 | this.options.parent | unsafe-jquery-plugin.js:68:45:68:63 | this.options.parent |
164+
| unsafe-jquery-plugin.js:68:7:68:25 | this.options.parent | unsafe-jquery-plugin.js:68:45:68:63 | this.options.parent |
142165
| unsafe-jquery-plugin.js:71:38:71:44 | options | unsafe-jquery-plugin.js:72:5:72:11 | options |
143166
| unsafe-jquery-plugin.js:71:38:71:44 | options | unsafe-jquery-plugin.js:72:5:72:11 | options |
144167
| unsafe-jquery-plugin.js:72:5:72:11 | options | unsafe-jquery-plugin.js:72:5:72:15 | options.foo |
@@ -165,26 +188,20 @@ edges
165188
| unsafe-jquery-plugin.js:92:5:92:11 | options | unsafe-jquery-plugin.js:85:14:85:14 | o |
166189
| unsafe-jquery-plugin.js:101:38:101:44 | options | unsafe-jquery-plugin.js:105:6:105:12 | options |
167190
| unsafe-jquery-plugin.js:101:38:101:44 | options | unsafe-jquery-plugin.js:105:6:105:12 | options |
168-
| unsafe-jquery-plugin.js:102:3:105:13 | options | unsafe-jquery-plugin.js:106:5:106:11 | options |
169191
| unsafe-jquery-plugin.js:102:3:105:13 | options | unsafe-jquery-plugin.js:107:5:107:11 | options |
170192
| unsafe-jquery-plugin.js:102:13:105:13 | $.exten ... ptions) | unsafe-jquery-plugin.js:102:3:105:13 | options |
171193
| unsafe-jquery-plugin.js:102:22:105:3 | {\\n\\t\\t\\tme ... in'\\n\\t\\t} | unsafe-jquery-plugin.js:102:13:105:13 | $.exten ... ptions) |
172194
| unsafe-jquery-plugin.js:105:6:105:12 | options | unsafe-jquery-plugin.js:102:13:105:13 | $.exten ... ptions) |
173195
| unsafe-jquery-plugin.js:105:6:105:12 | options | unsafe-jquery-plugin.js:102:22:105:3 | {\\n\\t\\t\\tme ... in'\\n\\t\\t} |
174-
| unsafe-jquery-plugin.js:106:5:106:11 | options | unsafe-jquery-plugin.js:106:5:106:16 | options.menu |
175-
| unsafe-jquery-plugin.js:106:5:106:11 | options | unsafe-jquery-plugin.js:106:5:106:16 | options.menu |
176196
| unsafe-jquery-plugin.js:107:5:107:11 | options | unsafe-jquery-plugin.js:107:5:107:18 | options.target |
177197
| unsafe-jquery-plugin.js:107:5:107:11 | options | unsafe-jquery-plugin.js:107:5:107:18 | options.target |
178198
| unsafe-jquery-plugin.js:114:38:114:44 | options | unsafe-jquery-plugin.js:115:51:115:57 | options |
179199
| unsafe-jquery-plugin.js:114:38:114:44 | options | unsafe-jquery-plugin.js:115:51:115:57 | options |
180-
| unsafe-jquery-plugin.js:115:3:115:58 | options | unsafe-jquery-plugin.js:116:5:116:11 | options |
181200
| unsafe-jquery-plugin.js:115:3:115:58 | options | unsafe-jquery-plugin.js:117:5:117:11 | options |
182201
| unsafe-jquery-plugin.js:115:13:115:58 | $.exten ... ptions) | unsafe-jquery-plugin.js:115:3:115:58 | options |
183202
| unsafe-jquery-plugin.js:115:22:115:23 | {} | unsafe-jquery-plugin.js:115:13:115:58 | $.exten ... ptions) |
184203
| unsafe-jquery-plugin.js:115:51:115:57 | options | unsafe-jquery-plugin.js:115:13:115:58 | $.exten ... ptions) |
185204
| unsafe-jquery-plugin.js:115:51:115:57 | options | unsafe-jquery-plugin.js:115:22:115:23 | {} |
186-
| unsafe-jquery-plugin.js:116:5:116:11 | options | unsafe-jquery-plugin.js:116:5:116:16 | options.menu |
187-
| unsafe-jquery-plugin.js:116:5:116:11 | options | unsafe-jquery-plugin.js:116:5:116:16 | options.menu |
188205
| unsafe-jquery-plugin.js:117:5:117:11 | options | unsafe-jquery-plugin.js:117:5:117:18 | options.target |
189206
| unsafe-jquery-plugin.js:117:5:117:11 | options | unsafe-jquery-plugin.js:117:5:117:18 | options.target |
190207
| unsafe-jquery-plugin.js:121:40:121:46 | options | unsafe-jquery-plugin.js:122:5:122:11 | options |
@@ -204,8 +221,17 @@ edges
204221
| unsafe-jquery-plugin.js:136:5:136:11 | options | unsafe-jquery-plugin.js:136:5:136:20 | options.viewport |
205222
| unsafe-jquery-plugin.js:136:5:136:20 | options.viewport | unsafe-jquery-plugin.js:136:5:136:29 | options ... elector |
206223
| unsafe-jquery-plugin.js:136:5:136:20 | options.viewport | unsafe-jquery-plugin.js:136:5:136:29 | options ... elector |
224+
| unsafe-jquery-plugin.js:153:38:153:44 | options | unsafe-jquery-plugin.js:154:16:154:22 | options |
225+
| unsafe-jquery-plugin.js:153:38:153:44 | options | unsafe-jquery-plugin.js:154:16:154:22 | options |
226+
| unsafe-jquery-plugin.js:153:38:153:44 | options | unsafe-jquery-plugin.js:156:3:156:9 | options |
227+
| unsafe-jquery-plugin.js:153:38:153:44 | options | unsafe-jquery-plugin.js:156:3:156:9 | options |
207228
| unsafe-jquery-plugin.js:153:38:153:44 | options | unsafe-jquery-plugin.js:157:44:157:50 | options |
208229
| unsafe-jquery-plugin.js:153:38:153:44 | options | unsafe-jquery-plugin.js:157:44:157:50 | options |
230+
| unsafe-jquery-plugin.js:154:16:154:22 | options | unsafe-jquery-plugin.js:154:16:154:29 | options.target |
231+
| unsafe-jquery-plugin.js:154:16:154:29 | options.target | unsafe-jquery-plugin.js:156:3:156:16 | options.target |
232+
| unsafe-jquery-plugin.js:154:16:154:29 | options.target | unsafe-jquery-plugin.js:157:44:157:57 | options.target |
233+
| unsafe-jquery-plugin.js:156:3:156:9 | options | unsafe-jquery-plugin.js:156:3:156:16 | options.target |
234+
| unsafe-jquery-plugin.js:156:3:156:16 | options.target | unsafe-jquery-plugin.js:157:44:157:57 | options.target |
209235
| unsafe-jquery-plugin.js:157:44:157:50 | options | unsafe-jquery-plugin.js:157:44:157:57 | options.target |
210236
| unsafe-jquery-plugin.js:157:44:157:57 | options.target | unsafe-jquery-plugin.js:157:44:157:59 | options.target.a |
211237
| unsafe-jquery-plugin.js:157:44:157:57 | options.target | unsafe-jquery-plugin.js:157:44:157:59 | options.target.a |
@@ -229,6 +255,7 @@ edges
229255
| unsafe-jquery-plugin.js:48:6:48:11 | target | unsafe-jquery-plugin.js:2:38:2:44 | options | unsafe-jquery-plugin.js:48:6:48:11 | target | Potential XSS vulnerability in the $@. | unsafe-jquery-plugin.js:2:19:63:2 | functio ... \\t\\t}\\n\\n\\t} | '$.fn.my_plugin' plugin |
230256
| unsafe-jquery-plugin.js:52:6:52:11 | target | unsafe-jquery-plugin.js:2:38:2:44 | options | unsafe-jquery-plugin.js:52:6:52:11 | target | Potential XSS vulnerability in the $@. | unsafe-jquery-plugin.js:2:19:63:2 | functio ... \\t\\t}\\n\\n\\t} | '$.fn.my_plugin' plugin |
231257
| unsafe-jquery-plugin.js:60:6:60:11 | target | unsafe-jquery-plugin.js:2:38:2:44 | options | unsafe-jquery-plugin.js:60:6:60:11 | target | Potential XSS vulnerability in the $@. | unsafe-jquery-plugin.js:2:19:63:2 | functio ... \\t\\t}\\n\\n\\t} | '$.fn.my_plugin' plugin |
258+
| unsafe-jquery-plugin.js:68:45:68:63 | this.options.parent | unsafe-jquery-plugin.js:65:47:65:53 | options | unsafe-jquery-plugin.js:68:45:68:63 | this.options.parent | Potential XSS vulnerability in the $@. | unsafe-jquery-plugin.js:65:19:69:2 | functio ... T OK\\n\\t} | '$.fn.my_plugin' plugin |
232259
| unsafe-jquery-plugin.js:72:5:72:23 | options.foo.bar.baz | unsafe-jquery-plugin.js:71:38:71:44 | options | unsafe-jquery-plugin.js:72:5:72:23 | options.foo.bar.baz | Potential XSS vulnerability in the $@. | unsafe-jquery-plugin.js:71:19:74:2 | functio ... / OK\\n\\t} | '$.fn.my_plugin' plugin |
233260
| unsafe-jquery-plugin.js:77:17:77:35 | options.foo.bar.baz | unsafe-jquery-plugin.js:76:38:76:44 | options | unsafe-jquery-plugin.js:77:17:77:35 | options.foo.bar.baz | Potential XSS vulnerability in the $@. | unsafe-jquery-plugin.js:76:19:78:2 | functio ... T OK\\n\\t} | '$.fn.my_plugin' plugin |
234261
| unsafe-jquery-plugin.js:90:6:90:6 | t | unsafe-jquery-plugin.js:84:38:84:44 | options | unsafe-jquery-plugin.js:90:6:90:6 | t | Potential XSS vulnerability in the $@. | unsafe-jquery-plugin.js:84:19:93:2 | functio ... ns);\\n\\t} | '$.fn.my_plugin' plugin |

‎javascript/ql/test/query-tests/Security/CWE-079/unsafe-jquery-plugin.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@
6565
$.fn.my_plugin = function my_plugin(element, options) {
6666
this.$element = $(element);
6767
this.options = $.extend({}, options);
68-
if (this.options.parent) this.$parent = $(this.options.parent) // NOT OK - but not flagged [INCONSISTENCY]
68+
if (this.options.parent) this.$parent = $(this.options.parent) // NOT OK
6969
};
7070

7171
$.fn.my_plugin = function my_plugin(options) {

0 commit comments

Comments
 (0)