Skip to content

Commit e6680de

Browse files
asgerfasgerf
committed
JS: Avoid use of LabeledSanitizerGuardNode in TaintedObject
Drive-by bugfix: Rename sanitizes -> blocksExpr. This fixes a bug that caused the sanitizer guard not to work in df2. The test output reflects the fact that the barrier guard works now.
1 parent 0ce1fe7 commit e6680de

File tree

2 files changed

+17
-9
lines changed

2 files changed

+17
-9
lines changed

‎javascript/ql/lib/semmle/javascript/security/TaintedObject.qll‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -81,18 +81,31 @@ module TaintedObject {
8181
/**
8282
* A sanitizer guard that blocks deep object taint.
8383
*/
84-
abstract class SanitizerGuard extends TaintTracking::LabeledSanitizerGuardNode {
84+
abstract class SanitizerGuard extends DataFlow::Node {
8585
/** Holds if this node blocks flow through `e`, provided it evaluates to `outcome`. */
8686
predicate blocksExpr(boolean outcome, Expr e) { none() }
8787

8888
/** Holds if this node blocks flow of `label` through `e`, provided it evaluates to `outcome`. */
8989
predicate blocksExpr(boolean outcome, Expr e, FlowLabel label) { none() }
9090

91-
override predicate sanitizes(boolean outcome, Expr e, FlowLabel label) {
91+
/** DEPRECATED. Use `blocksExpr` instead. */
92+
deprecated predicate sanitizes(boolean outcome, Expr e, FlowLabel label) {
9293
this.blocksExpr(outcome, e, label)
9394
}
9495

95-
override predicate sanitizes(boolean outcome, Expr e) { this.blocksExpr(outcome, e) }
96+
/** DEPRECATED. Use `blocksExpr` instead. */
97+
deprecated predicate sanitizes(boolean outcome, Expr e) { this.blocksExpr(outcome, e) }
98+
}
99+
100+
deprecated private class SanitizerGuardLegacy extends TaintTracking::LabeledSanitizerGuardNode instanceof SanitizerGuard
101+
{
102+
deprecated override predicate sanitizes(boolean outcome, Expr e, FlowLabel label) {
103+
SanitizerGuard.super.sanitizes(outcome, e, label)
104+
}
105+
106+
deprecated override predicate sanitizes(boolean outcome, Expr e) {
107+
SanitizerGuard.super.sanitizes(outcome, e)
108+
}
96109
}
97110

98111
/**
@@ -148,7 +161,7 @@ module TaintedObject {
148161
.getACall()
149162
}
150163

151-
override predicate sanitizes(boolean outcome, Expr e, FlowLabel lbl) {
164+
override predicate blocksExpr(boolean outcome, Expr e, FlowLabel lbl) {
152165
e = super.getAnArgument().asExpr() and outcome = true and lbl = label()
153166
}
154167
}

‎javascript/ql/test/query-tests/Security/CWE-089/untyped/SqlInjection.expected‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,6 @@ nodes
198198
| mongoose.js:130:16:130:26 | { _id: id } | semmle.label | { _id: id } |
199199
| mongoose.js:130:23:130:24 | id | semmle.label | id |
200200
| mongoose.js:133:38:133:42 | query | semmle.label | query |
201-
| mongoose.js:134:30:134:34 | query | semmle.label | query |
202201
| mongoose.js:136:30:136:34 | query | semmle.label | query |
203202
| mongooseJsonParse.js:19:11:19:20 | query | semmle.label | query |
204203
| mongooseJsonParse.js:19:19:19:20 | {} | semmle.label | {} |
@@ -453,7 +452,6 @@ edges
453452
| mongoose.js:20:8:20:17 | query | mongoose.js:111:14:111:18 | query | provenance | |
454453
| mongoose.js:20:8:20:17 | query | mongoose.js:113:31:113:35 | query | provenance | |
455454
| mongoose.js:20:8:20:17 | query | mongoose.js:133:38:133:42 | query | provenance | |
456-
| mongoose.js:20:8:20:17 | query | mongoose.js:134:30:134:34 | query | provenance | |
457455
| mongoose.js:20:8:20:17 | query | mongoose.js:136:30:136:34 | query | provenance | |
458456
| mongoose.js:20:16:20:17 | {} | mongoose.js:20:8:20:17 | query | provenance | |
459457
| mongoose.js:21:2:21:6 | query | mongoose.js:24:22:24:26 | query | provenance | |
@@ -498,7 +496,6 @@ edges
498496
| mongoose.js:21:16:21:29 | req.body.title | mongoose.js:111:14:111:18 | query | provenance | Config |
499497
| mongoose.js:21:16:21:29 | req.body.title | mongoose.js:113:31:113:35 | query | provenance | Config |
500498
| mongoose.js:21:16:21:29 | req.body.title | mongoose.js:133:38:133:42 | query | provenance | Config |
501-
| mongoose.js:21:16:21:29 | req.body.title | mongoose.js:134:30:134:34 | query | provenance | Config |
502499
| mongoose.js:21:16:21:29 | req.body.title | mongoose.js:136:30:136:34 | query | provenance | Config |
503500
| mongoose.js:24:22:24:26 | query | mongoose.js:24:21:24:27 | [query] | provenance | Config |
504501
| mongoose.js:24:22:24:26 | query | mongoose.js:27:17:27:21 | query | provenance | |
@@ -555,7 +552,6 @@ edges
555552
| mongoose.js:115:25:115:45 | cond | mongoose.js:129:21:129:24 | cond | provenance | |
556553
| mongoose.js:115:32:115:45 | req.query.cond | mongoose.js:115:25:115:45 | cond | provenance | |
557554
| mongoose.js:130:23:130:24 | id | mongoose.js:130:16:130:26 | { _id: id } | provenance | Config |
558-
| mongoose.js:133:38:133:42 | query | mongoose.js:134:30:134:34 | query | provenance | |
559555
| mongoose.js:133:38:133:42 | query | mongoose.js:136:30:136:34 | query | provenance | |
560556
| mongooseJsonParse.js:19:11:19:20 | query | mongooseJsonParse.js:23:19:23:23 | query | provenance | |
561557
| mongooseJsonParse.js:19:19:19:20 | {} | mongooseJsonParse.js:19:11:19:20 | query | provenance | |
@@ -718,7 +714,6 @@ subpaths
718714
| mongoose.js:128:22:128:25 | cond | mongoose.js:115:32:115:45 | req.query.cond | mongoose.js:128:22:128:25 | cond | This query object depends on a $@. | mongoose.js:115:32:115:45 | req.query.cond | user-provided value |
719715
| mongoose.js:129:21:129:24 | cond | mongoose.js:115:32:115:45 | req.query.cond | mongoose.js:129:21:129:24 | cond | This query object depends on a $@. | mongoose.js:115:32:115:45 | req.query.cond | user-provided value |
720716
| mongoose.js:130:16:130:26 | { _id: id } | mongoose.js:115:11:115:22 | req.query.id | mongoose.js:130:16:130:26 | { _id: id } | This query object depends on a $@. | mongoose.js:115:11:115:22 | req.query.id | user-provided value |
721-
| mongoose.js:134:30:134:34 | query | mongoose.js:21:16:21:23 | req.body | mongoose.js:134:30:134:34 | query | This query object depends on a $@. | mongoose.js:21:16:21:23 | req.body | user-provided value |
722717
| mongoose.js:136:30:136:34 | query | mongoose.js:21:16:21:23 | req.body | mongoose.js:136:30:136:34 | query | This query object depends on a $@. | mongoose.js:21:16:21:23 | req.body | user-provided value |
723718
| mongooseJsonParse.js:23:19:23:23 | query | mongooseJsonParse.js:20:30:20:43 | req.query.data | mongooseJsonParse.js:23:19:23:23 | query | This query object depends on a $@. | mongooseJsonParse.js:20:30:20:43 | req.query.data | user-provided value |
724719
| mongooseModelClient.js:11:16:11:24 | { id: v } | mongooseModelClient.js:10:22:10:29 | req.body | mongooseModelClient.js:11:16:11:24 | { id: v } | This query object depends on a $@. | mongooseModelClient.js:10:22:10:29 | req.body | user-provided value |

0 commit comments

Comments
 (0)