Skip to content

Commit de70593

Browse files
committed
add localFieldStep as a DataFlow::SharedFlowStep, and support writes on nested properties
1 parent 6cbcc7a commit de70593

File tree

3 files changed

+21
-2
lines changed

3 files changed

+21
-2
lines changed

javascript/ql/lib/semmle/javascript/dataflow/DataFlow.qll

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1591,13 +1591,22 @@ module DataFlow {
15911591
*/
15921592
predicate localFieldStep(DataFlow::Node pred, DataFlow::Node succ) {
15931593
exists(ClassNode cls, string prop |
1594-
pred = cls.getADirectSuperClass*().getAReceiverNode().getAPropertyWrite(prop).getRhs() or
1594+
pred = cls.getADirectSuperClass*().getAReceiverNode().getAPropertyWrite(prop).getRhs()
1595+
or
1596+
// add support for writes on nested properties
1597+
pred = cls.getADirectSuperClass*().getAReceiverNode().getAPropertyRead(prop) and
1598+
pred = any(DataFlow::PropRef ref).getBase()
1599+
or
15951600
pred = cls.getInstanceMethod(prop)
15961601
|
15971602
succ = cls.getAReceiverNode().getAPropertyRead(prop)
15981603
)
15991604
}
16001605

1606+
private class LocalFieldStep extends DataFlow::SharedFlowStep {
1607+
override predicate step(DataFlow::Node pred, DataFlow::Node succ) { localFieldStep(pred, succ) }
1608+
}
1609+
16011610
predicate argumentPassingStep = FlowSteps::argumentPassing/4;
16021611

16031612
/**

javascript/ql/test/query-tests/Security/CWE-078/UnsafeShellCommandConstruction.expected

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,10 @@ nodes
166166
| lib/lib.js:277:23:277:26 | opts |
167167
| lib/lib.js:277:23:277:30 | opts.bla |
168168
| lib/lib.js:277:23:277:30 | opts.bla |
169+
| lib/lib.js:279:19:279:22 | opts |
170+
| lib/lib.js:279:19:279:26 | opts.bla |
171+
| lib/lib.js:281:23:281:35 | this.opts.bla |
172+
| lib/lib.js:281:23:281:35 | this.opts.bla |
169173
| lib/lib.js:307:39:307:42 | name |
170174
| lib/lib.js:307:39:307:42 | name |
171175
| lib/lib.js:308:23:308:26 | name |
@@ -468,8 +472,13 @@ edges
468472
| lib/lib.js:268:22:268:24 | obj | lib/lib.js:268:22:268:32 | obj.version |
469473
| lib/lib.js:276:8:276:11 | opts | lib/lib.js:277:23:277:26 | opts |
470474
| lib/lib.js:276:8:276:11 | opts | lib/lib.js:277:23:277:26 | opts |
475+
| lib/lib.js:276:8:276:11 | opts | lib/lib.js:279:19:279:22 | opts |
476+
| lib/lib.js:276:8:276:11 | opts | lib/lib.js:279:19:279:22 | opts |
471477
| lib/lib.js:277:23:277:26 | opts | lib/lib.js:277:23:277:30 | opts.bla |
472478
| lib/lib.js:277:23:277:26 | opts | lib/lib.js:277:23:277:30 | opts.bla |
479+
| lib/lib.js:279:19:279:22 | opts | lib/lib.js:279:19:279:26 | opts.bla |
480+
| lib/lib.js:279:19:279:26 | opts.bla | lib/lib.js:281:23:281:35 | this.opts.bla |
481+
| lib/lib.js:279:19:279:26 | opts.bla | lib/lib.js:281:23:281:35 | this.opts.bla |
473482
| lib/lib.js:307:39:307:42 | name | lib/lib.js:308:23:308:26 | name |
474483
| lib/lib.js:307:39:307:42 | name | lib/lib.js:308:23:308:26 | name |
475484
| lib/lib.js:307:39:307:42 | name | lib/lib.js:308:23:308:26 | name |
@@ -636,6 +645,7 @@ edges
636645
| lib/lib.js:261:11:261:33 | "rm -rf ... + name | lib/lib.js:257:35:257:38 | name | lib/lib.js:261:30:261:33 | name | $@ based on $@ is later used in $@. | lib/lib.js:261:11:261:33 | "rm -rf ... + name | String concatenation | lib/lib.js:257:35:257:38 | name | library input | lib/lib.js:261:3:261:34 | cp.exec ... + name) | shell command |
637646
| lib/lib.js:268:10:268:32 | "rm -rf ... version | lib/lib.js:267:46:267:48 | obj | lib/lib.js:268:22:268:32 | obj.version | $@ based on $@ is later used in $@. | lib/lib.js:268:10:268:32 | "rm -rf ... version | String concatenation | lib/lib.js:267:46:267:48 | obj | library input | lib/lib.js:268:2:268:33 | cp.exec ... ersion) | shell command |
638647
| lib/lib.js:277:11:277:30 | "rm -rf " + opts.bla | lib/lib.js:276:8:276:11 | opts | lib/lib.js:277:23:277:30 | opts.bla | $@ based on $@ is later used in $@. | lib/lib.js:277:11:277:30 | "rm -rf " + opts.bla | String concatenation | lib/lib.js:276:8:276:11 | opts | library input | lib/lib.js:277:3:277:31 | cp.exec ... ts.bla) | shell command |
648+
| lib/lib.js:281:11:281:35 | "rm -rf ... pts.bla | lib/lib.js:276:8:276:11 | opts | lib/lib.js:281:23:281:35 | this.opts.bla | $@ based on $@ is later used in $@. | lib/lib.js:281:11:281:35 | "rm -rf ... pts.bla | String concatenation | lib/lib.js:276:8:276:11 | opts | library input | lib/lib.js:281:3:281:36 | cp.exec ... ts.bla) | shell command |
639649
| lib/lib.js:308:11:308:26 | "rm -rf " + name | lib/lib.js:307:39:307:42 | name | lib/lib.js:308:23:308:26 | name | $@ based on $@ is later used in $@. | lib/lib.js:308:11:308:26 | "rm -rf " + name | String concatenation | lib/lib.js:307:39:307:42 | name | library input | lib/lib.js:308:3:308:27 | cp.exec ... + name) | shell command |
640650
| lib/lib.js:315:10:315:25 | "rm -rf " + name | lib/lib.js:314:40:314:43 | name | lib/lib.js:315:22:315:25 | name | $@ based on $@ is later used in $@. | lib/lib.js:315:10:315:25 | "rm -rf " + name | String concatenation | lib/lib.js:314:40:314:43 | name | library input | lib/lib.js:315:2:315:26 | cp.exec ... + name) | shell command |
641651
| lib/lib.js:320:11:320:26 | "rm -rf " + name | lib/lib.js:314:40:314:43 | name | lib/lib.js:320:23:320:26 | name | $@ based on $@ is later used in $@. | lib/lib.js:320:11:320:26 | "rm -rf " + name | String concatenation | lib/lib.js:314:40:314:43 | name | library input | lib/lib.js:320:3:320:27 | cp.exec ... + name) | shell command |

javascript/ql/test/query-tests/Security/CWE-078/lib/lib.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -278,7 +278,7 @@ module.exports.Foo = class Foo {
278278
this.opts = {};
279279
this.opts.bla = opts.bla
280280

281-
cp.exec("rm -rf " + this.opts.bla); // NOT OK - but FN [INCONSISTENCY]
281+
cp.exec("rm -rf " + this.opts.bla); // NOT OK
282282
}
283283
}
284284

0 commit comments

Comments
 (0)