Skip to content

Commit 943bdec

Browse files
erik-krogherik-krogh
committed
make appliesTo recursive
1 parent 9549cac commit 943bdec

File tree

3 files changed

+74
-15
lines changed

3 files changed

+74
-15
lines changed

‎javascript/ql/lib/semmle/javascript/dataflow/Configuration.qll

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ abstract class Configuration extends string {
161161
*/
162162
predicate isBarrier(DataFlow::Node node) {
163163
exists(BarrierGuardNode guard |
164-
isBarrierGuardInternal(guard) and
164+
isBarrierGuardInternal(this, guard) and
165165
barrierGuardBlocksNode(guard, node, "")
166166
)
167167
}
@@ -181,7 +181,7 @@ abstract class Configuration extends string {
181181
*/
182182
predicate isLabeledBarrier(DataFlow::Node node, FlowLabel lbl) {
183183
exists(BarrierGuardNode guard |
184-
isBarrierGuardInternal(guard) and
184+
isBarrierGuardInternal(this, guard) and
185185
barrierGuardBlocksNode(guard, node, lbl)
186186
)
187187
or
@@ -198,17 +198,6 @@ abstract class Configuration extends string {
198198
*/
199199
predicate isBarrierGuard(BarrierGuardNode guard) { none() }
200200

201-
/**
202-
* Holds if `guard` is a barrier guard for this configuration, added through
203-
* `isBarrierGuard` or `AdditionalBarrierGuardNode`.
204-
*/
205-
pragma[nomagic]
206-
private predicate isBarrierGuardInternal(BarrierGuardNode guard) {
207-
isBarrierGuard(guard)
208-
or
209-
guard.(AdditionalBarrierGuardNode).appliesTo(this)
210-
}
211-
212201
/**
213202
* Holds if data may flow from `source` to `sink` for this configuration.
214203
*/
@@ -267,6 +256,17 @@ abstract class Configuration extends string {
267256
}
268257
}
269258

259+
/**
260+
* Holds if `guard` is a barrier guard for this configuration, added through
261+
* `isBarrierGuard` or `AdditionalBarrierGuardNode`.
262+
*/
263+
pragma[nomagic]
264+
private predicate isBarrierGuardInternal(Configuration cfg, BarrierGuardNode guard) {
265+
cfg.isBarrierGuard(guard)
266+
or
267+
guard.(AdditionalBarrierGuardNode).appliesTo(cfg)
268+
}
269+
270270
/**
271271
* A label describing the kind of information tracked by a flow configuration.
272272
*
@@ -1981,7 +1981,7 @@ private class BarrierGuardFunction extends Function {
19811981
/**
19821982
* Holds if this function applies to the flow in `cfg`.
19831983
*/
1984-
predicate appliesTo(Configuration cfg) { cfg.isBarrierGuard(guard) }
1984+
predicate appliesTo(Configuration cfg) { isBarrierGuardInternal(cfg, guard) }
19851985
}
19861986

19871987
/**
@@ -2034,7 +2034,7 @@ private class CallAgainstEqualityCheck extends AdditionalBarrierGuardNode {
20342034
)
20352035
}
20362036

2037-
override predicate appliesTo(Configuration cfg) { cfg.isBarrierGuard(prev) }
2037+
override predicate appliesTo(Configuration cfg) { isBarrierGuardInternal(cfg, prev) }
20382038
}
20392039

20402040
/**

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

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -311,6 +311,14 @@ nodes
311311
| lib/lib.js:590:29:590:32 | name |
312312
| lib/lib.js:593:25:593:28 | name |
313313
| lib/lib.js:593:25:593:28 | name |
314+
| lib/lib.js:608:42:608:45 | name |
315+
| lib/lib.js:608:42:608:45 | name |
316+
| lib/lib.js:609:22:609:25 | name |
317+
| lib/lib.js:609:22:609:25 | name |
318+
| lib/lib.js:626:29:626:32 | name |
319+
| lib/lib.js:626:29:626:32 | name |
320+
| lib/lib.js:629:25:629:28 | name |
321+
| lib/lib.js:629:25:629:28 | name |
314322
| lib/subLib2/compiled-file.ts:3:26:3:29 | name |
315323
| lib/subLib2/compiled-file.ts:3:26:3:29 | name |
316324
| lib/subLib2/compiled-file.ts:4:25:4:28 | name |
@@ -729,6 +737,18 @@ edges
729737
| lib/lib.js:572:41:572:44 | name | lib/lib.js:593:25:593:28 | name |
730738
| lib/lib.js:572:41:572:44 | name | lib/lib.js:593:25:593:28 | name |
731739
| lib/lib.js:572:41:572:44 | name | lib/lib.js:593:25:593:28 | name |
740+
| lib/lib.js:608:42:608:45 | name | lib/lib.js:609:22:609:25 | name |
741+
| lib/lib.js:608:42:608:45 | name | lib/lib.js:609:22:609:25 | name |
742+
| lib/lib.js:608:42:608:45 | name | lib/lib.js:609:22:609:25 | name |
743+
| lib/lib.js:608:42:608:45 | name | lib/lib.js:609:22:609:25 | name |
744+
| lib/lib.js:608:42:608:45 | name | lib/lib.js:626:29:626:32 | name |
745+
| lib/lib.js:608:42:608:45 | name | lib/lib.js:626:29:626:32 | name |
746+
| lib/lib.js:608:42:608:45 | name | lib/lib.js:626:29:626:32 | name |
747+
| lib/lib.js:608:42:608:45 | name | lib/lib.js:626:29:626:32 | name |
748+
| lib/lib.js:608:42:608:45 | name | lib/lib.js:629:25:629:28 | name |
749+
| lib/lib.js:608:42:608:45 | name | lib/lib.js:629:25:629:28 | name |
750+
| lib/lib.js:608:42:608:45 | name | lib/lib.js:629:25:629:28 | name |
751+
| lib/lib.js:608:42:608:45 | name | lib/lib.js:629:25:629:28 | name |
732752
| lib/subLib2/compiled-file.ts:3:26:3:29 | name | lib/subLib2/compiled-file.ts:4:25:4:28 | name |
733753
| lib/subLib2/compiled-file.ts:3:26:3:29 | name | lib/subLib2/compiled-file.ts:4:25:4:28 | name |
734754
| lib/subLib2/compiled-file.ts:3:26:3:29 | name | lib/subLib2/compiled-file.ts:4:25:4:28 | name |
@@ -856,6 +876,9 @@ edges
856876
| lib/lib.js:579:13:579:28 | "rm -rf " + name | lib/lib.js:572:41:572:44 | name | lib/lib.js:579:25:579:28 | name | This string concatenation which depends on $@ is later used in a $@. | lib/lib.js:572:41:572:44 | name | library input | lib/lib.js:579:5:579:29 | cp.exec ... + name) | shell command |
857877
| lib/lib.js:590:17:590:32 | "rm -rf " + name | lib/lib.js:572:41:572:44 | name | lib/lib.js:590:29:590:32 | name | This string concatenation which depends on $@ is later used in a $@. | lib/lib.js:572:41:572:44 | name | library input | lib/lib.js:590:9:590:33 | cp.exec ... + name) | shell command |
858878
| lib/lib.js:593:13:593:28 | "rm -rf " + name | lib/lib.js:572:41:572:44 | name | lib/lib.js:593:25:593:28 | name | This string concatenation which depends on $@ is later used in a $@. | lib/lib.js:572:41:572:44 | name | library input | lib/lib.js:593:5:593:29 | cp.exec ... + name) | shell command |
879+
| lib/lib.js:609:10:609:25 | "rm -rf " + name | lib/lib.js:608:42:608:45 | name | lib/lib.js:609:22:609:25 | name | This string concatenation which depends on $@ is later used in a $@. | lib/lib.js:608:42:608:45 | name | library input | lib/lib.js:609:2:609:26 | cp.exec ... + name) | shell command |
880+
| lib/lib.js:626:17:626:32 | "rm -rf " + name | lib/lib.js:608:42:608:45 | name | lib/lib.js:626:29:626:32 | name | This string concatenation which depends on $@ is later used in a $@. | lib/lib.js:608:42:608:45 | name | library input | lib/lib.js:626:9:626:33 | cp.exec ... + name) | shell command |
881+
| lib/lib.js:629:13:629:28 | "rm -rf " + name | lib/lib.js:608:42:608:45 | name | lib/lib.js:629:25:629:28 | name | This string concatenation which depends on $@ is later used in a $@. | lib/lib.js:608:42:608:45 | name | library input | lib/lib.js:629:5:629:29 | cp.exec ... + name) | shell command |
859882
| lib/subLib2/compiled-file.ts:4:13:4:28 | "rm -rf " + name | lib/subLib2/compiled-file.ts:3:26:3:29 | name | lib/subLib2/compiled-file.ts:4:25:4:28 | name | This string concatenation which depends on $@ is later used in a $@. | lib/subLib2/compiled-file.ts:3:26:3:29 | name | library input | lib/subLib2/compiled-file.ts:4:5:4:29 | cp.exec ... + name) | shell command |
860883
| lib/subLib2/special-file.js:4:10:4:25 | "rm -rf " + name | lib/subLib2/special-file.js:3:28:3:31 | name | lib/subLib2/special-file.js:4:22:4:25 | name | This string concatenation which depends on $@ is later used in a $@. | lib/subLib2/special-file.js:3:28:3:31 | name | library input | lib/subLib2/special-file.js:4:2:4:26 | cp.exec ... + name) | shell command |
861884
| lib/subLib3/my-file.ts:4:10:4:25 | "rm -rf " + name | lib/subLib3/my-file.ts:3:28:3:31 | name | lib/subLib3/my-file.ts:4:22:4:25 | name | This string concatenation which depends on $@ is later used in a $@. | lib/subLib3/my-file.ts:3:28:3:31 | name | library input | lib/subLib3/my-file.ts:4:2:4:26 | cp.exec ... + name) | shell command |

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

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -592,3 +592,39 @@ module.exports.safeWithBool = function (name) {
592592

593593
cp.exec("rm -rf " + name); // NOT OK
594594
}
595+
596+
function indirectThing(name) {
597+
return isSafeName(name);
598+
}
599+
600+
function indirectThing2(name) {
601+
return isSafeName(name) === true;
602+
}
603+
604+
function moreIndirect(name) {
605+
return indirectThing2(name) !== false;
606+
}
607+
608+
module.exports.veryIndeirect = function (name) {
609+
cp.exec("rm -rf " + name); // NOT OK
610+
611+
if (indirectThing(name)) {
612+
cp.exec("rm -rf " + name); // OK
613+
}
614+
615+
if (indirectThing2(name)) {
616+
cp.exec("rm -rf " + name); // OK
617+
}
618+
619+
if (moreIndirect(name)) {
620+
cp.exec("rm -rf " + name); // OK
621+
}
622+
623+
if (moreIndirect(name) !== false) {
624+
cp.exec("rm -rf " + name); // OK
625+
} else {
626+
cp.exec("rm -rf " + name); // NOT OK
627+
}
628+
629+
cp.exec("rm -rf " + name); // NOT OK
630+
}

0 commit comments

Comments
 (0)