Skip to content

Commit 550c578

Browse files
erik-krogherik-krogh
committed
use MemberShipTest in TaintedPath
1 parent d513e6c commit 550c578

File tree

4 files changed

+27
-102
lines changed

4 files changed

+27
-102
lines changed

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1814,3 +1814,19 @@ class VarAccessBarrier extends DataFlow::Node {
18141814
)
18151815
}
18161816
}
1817+
1818+
/**
1819+
* A check of the form `whitelist.includes(x)` or equivalent, which sanitizes `x` in its "then" branch.
1820+
*
1821+
* Can be added to `isBarrierGuard` in a data-flow configuration to block flow through such checks.
1822+
*/
1823+
class MembershipTestBarrierGuard extends BarrierGuardNode {
1824+
MembershipCandidate candidate;
1825+
1826+
MembershipTestBarrierGuard() { this = candidate.getTest() }
1827+
1828+
override predicate blocks(boolean outcome, Expr e) {
1829+
candidate = e.flow() and
1830+
candidate.getTestPolarity() = outcome
1831+
}
1832+
}

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -370,6 +370,15 @@ module TaintedPath {
370370
}
371371
}
372372

373+
/**
374+
* A check of the form `whitelist.includes(x)` or equivalent, which sanitizes `x` in its "then" branch.
375+
*/
376+
class MembershipTestBarrierGuard extends BarrierGuardNode, DataFlow::MembershipTestBarrierGuard {
377+
override predicate blocks(boolean outcome, Expr e) {
378+
DataFlow::MembershipTestBarrierGuard.super.blocks(outcome, e)
379+
}
380+
}
381+
373382
/**
374383
* A check of form `x.startsWith(dir)` that sanitizes normalized absolute paths, since it is then
375384
* known to be in a subdirectory of `dir`.

‎javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/TaintedPath.expected

Lines changed: 0 additions & 100 deletions
Original file line numberDiff line numberDiff line change
@@ -211,40 +211,6 @@ nodes
211211
| TaintedPath.js:24:33:24:36 | path |
212212
| TaintedPath.js:24:33:24:36 | path |
213213
| TaintedPath.js:24:33:24:36 | path |
214-
| TaintedPath.js:27:31:27:34 | path |
215-
| TaintedPath.js:27:31:27:34 | path |
216-
| TaintedPath.js:27:31:27:34 | path |
217-
| TaintedPath.js:27:31:27:34 | path |
218-
| TaintedPath.js:27:31:27:34 | path |
219-
| TaintedPath.js:27:31:27:34 | path |
220-
| TaintedPath.js:27:31:27:34 | path |
221-
| TaintedPath.js:27:31:27:34 | path |
222-
| TaintedPath.js:27:31:27:34 | path |
223-
| TaintedPath.js:27:31:27:34 | path |
224-
| TaintedPath.js:27:31:27:34 | path |
225-
| TaintedPath.js:27:31:27:34 | path |
226-
| TaintedPath.js:27:31:27:34 | path |
227-
| TaintedPath.js:27:31:27:34 | path |
228-
| TaintedPath.js:27:31:27:34 | path |
229-
| TaintedPath.js:27:31:27:34 | path |
230-
| TaintedPath.js:27:31:27:34 | path |
231-
| TaintedPath.js:30:31:30:34 | path |
232-
| TaintedPath.js:30:31:30:34 | path |
233-
| TaintedPath.js:30:31:30:34 | path |
234-
| TaintedPath.js:30:31:30:34 | path |
235-
| TaintedPath.js:30:31:30:34 | path |
236-
| TaintedPath.js:30:31:30:34 | path |
237-
| TaintedPath.js:30:31:30:34 | path |
238-
| TaintedPath.js:30:31:30:34 | path |
239-
| TaintedPath.js:30:31:30:34 | path |
240-
| TaintedPath.js:30:31:30:34 | path |
241-
| TaintedPath.js:30:31:30:34 | path |
242-
| TaintedPath.js:30:31:30:34 | path |
243-
| TaintedPath.js:30:31:30:34 | path |
244-
| TaintedPath.js:30:31:30:34 | path |
245-
| TaintedPath.js:30:31:30:34 | path |
246-
| TaintedPath.js:30:31:30:34 | path |
247-
| TaintedPath.js:30:31:30:34 | path |
248214
| TaintedPath.js:33:31:33:34 | path |
249215
| TaintedPath.js:33:31:33:34 | path |
250216
| TaintedPath.js:33:31:33:34 | path |
@@ -2968,70 +2934,6 @@ edges
29682934
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:24:33:24:36 | path |
29692935
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:24:33:24:36 | path |
29702936
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:24:33:24:36 | path |
2971-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:27:31:27:34 | path |
2972-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:27:31:27:34 | path |
2973-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:27:31:27:34 | path |
2974-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:27:31:27:34 | path |
2975-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:27:31:27:34 | path |
2976-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:27:31:27:34 | path |
2977-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:27:31:27:34 | path |
2978-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:27:31:27:34 | path |
2979-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:27:31:27:34 | path |
2980-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:27:31:27:34 | path |
2981-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:27:31:27:34 | path |
2982-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:27:31:27:34 | path |
2983-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:27:31:27:34 | path |
2984-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:27:31:27:34 | path |
2985-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:27:31:27:34 | path |
2986-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:27:31:27:34 | path |
2987-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:27:31:27:34 | path |
2988-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:27:31:27:34 | path |
2989-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:27:31:27:34 | path |
2990-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:27:31:27:34 | path |
2991-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:27:31:27:34 | path |
2992-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:27:31:27:34 | path |
2993-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:27:31:27:34 | path |
2994-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:27:31:27:34 | path |
2995-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:27:31:27:34 | path |
2996-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:27:31:27:34 | path |
2997-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:27:31:27:34 | path |
2998-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:27:31:27:34 | path |
2999-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:27:31:27:34 | path |
3000-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:27:31:27:34 | path |
3001-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:27:31:27:34 | path |
3002-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:27:31:27:34 | path |
3003-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:30:31:30:34 | path |
3004-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:30:31:30:34 | path |
3005-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:30:31:30:34 | path |
3006-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:30:31:30:34 | path |
3007-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:30:31:30:34 | path |
3008-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:30:31:30:34 | path |
3009-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:30:31:30:34 | path |
3010-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:30:31:30:34 | path |
3011-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:30:31:30:34 | path |
3012-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:30:31:30:34 | path |
3013-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:30:31:30:34 | path |
3014-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:30:31:30:34 | path |
3015-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:30:31:30:34 | path |
3016-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:30:31:30:34 | path |
3017-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:30:31:30:34 | path |
3018-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:30:31:30:34 | path |
3019-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:30:31:30:34 | path |
3020-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:30:31:30:34 | path |
3021-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:30:31:30:34 | path |
3022-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:30:31:30:34 | path |
3023-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:30:31:30:34 | path |
3024-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:30:31:30:34 | path |
3025-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:30:31:30:34 | path |
3026-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:30:31:30:34 | path |
3027-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:30:31:30:34 | path |
3028-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:30:31:30:34 | path |
3029-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:30:31:30:34 | path |
3030-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:30:31:30:34 | path |
3031-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:30:31:30:34 | path |
3032-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:30:31:30:34 | path |
3033-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:30:31:30:34 | path |
3034-
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:30:31:30:34 | path |
30352937
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:33:31:33:34 | path |
30362938
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:33:31:33:34 | path |
30372939
| TaintedPath.js:9:7:9:48 | path | TaintedPath.js:33:31:33:34 | path |
@@ -6671,8 +6573,6 @@ edges
66716573
| TaintedPath.js:18:33:18:36 | path | TaintedPath.js:9:24:9:30 | req.url | TaintedPath.js:18:33:18:36 | path | This path depends on $@. | TaintedPath.js:9:24:9:30 | req.url | a user-provided value |
66726574
| TaintedPath.js:21:33:21:36 | path | TaintedPath.js:9:24:9:30 | req.url | TaintedPath.js:21:33:21:36 | path | This path depends on $@. | TaintedPath.js:9:24:9:30 | req.url | a user-provided value |
66736575
| TaintedPath.js:24:33:24:36 | path | TaintedPath.js:9:24:9:30 | req.url | TaintedPath.js:24:33:24:36 | path | This path depends on $@. | TaintedPath.js:9:24:9:30 | req.url | a user-provided value |
6674-
| TaintedPath.js:27:31:27:34 | path | TaintedPath.js:9:24:9:30 | req.url | TaintedPath.js:27:31:27:34 | path | This path depends on $@. | TaintedPath.js:9:24:9:30 | req.url | a user-provided value |
6675-
| TaintedPath.js:30:31:30:34 | path | TaintedPath.js:9:24:9:30 | req.url | TaintedPath.js:30:31:30:34 | path | This path depends on $@. | TaintedPath.js:9:24:9:30 | req.url | a user-provided value |
66766576
| TaintedPath.js:33:31:33:34 | path | TaintedPath.js:9:24:9:30 | req.url | TaintedPath.js:33:31:33:34 | path | This path depends on $@. | TaintedPath.js:9:24:9:30 | req.url | a user-provided value |
66776577
| TaintedPath.js:42:29:42:52 | pathMod ... e(path) | TaintedPath.js:38:20:38:26 | req.url | TaintedPath.js:42:29:42:52 | pathMod ... e(path) | This path depends on $@. | TaintedPath.js:38:20:38:26 | req.url | a user-provided value |
66786578
| TaintedPath.js:46:29:46:49 | pathMod ... n(path) | TaintedPath.js:38:20:38:26 | req.url | TaintedPath.js:46:29:46:49 | pathMod ... n(path) | This path depends on $@. | TaintedPath.js:38:20:38:26 | req.url | a user-provided value |

‎javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/TaintedPath.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,10 @@ var server = http.createServer(function(req, res) {
2424
res.write(fs.readFileSync(path)); // BAD: Insufficient sanitisation
2525

2626
if (path === 'foo.txt')
27-
res.write(fs.readFileSync(path)); // GOOD: Path is compared to white-list [INCONSISTENCY]
27+
res.write(fs.readFileSync(path)); // GOOD: Path is compared to white-list
2828

2929
if (path === 'foo.txt' || path === 'bar.txt')
30-
res.write(fs.readFileSync(path)); // GOOD: Path is compared to white-list [INCONSISTENCY]
30+
res.write(fs.readFileSync(path)); // GOOD: Path is compared to white-list
3131

3232
if (path === 'foo.txt' || path === 'bar.txt' || someOpaqueCondition())
3333
res.write(fs.readFileSync(path)); // BAD: Path is incompletely compared to white-list

0 commit comments

Comments
 (0)