Skip to content

JS: Support for matchAll #17910

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Napalys merged 14 commits into from
Nov 14, 2024
Merged

JS: Support for matchAll #17910

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
ccee34d
Added support for matchAll in CWE-020 including new test cases
Napalys Nov 5, 2024
b239bfa
Added tests forIncompleteHostnameRegExp and normalizedPaths using mat…
Napalys Nov 5, 2024
7825a46
Merge branch 'github:main' into napalys/matchAll-support
Napalys Nov 5, 2024
5e8b1b0
Update javascript/ql/src/Security/CWE-020/MissingRegExpAnchor.ql
Napalys Nov 5, 2024
4106663
Added tests for regex sanitization to identify false positives matchAll
Napalys Nov 7, 2024
449cee9
Fixes false positives from commit 445552d3b53ec9592e8e3892cb337d1004b…
Napalys Nov 7, 2024
42600c9
Added tests which shows false positive SSRF via matchAll
Napalys Nov 7, 2024
514375d
Fixes false positives from commit 42600c93ffad33fce4975542e11f32d45fd…
Napalys Nov 7, 2024
a4fe728
Added matchAll test which is not marked as vulnurability by CodeQL
Napalys Nov 7, 2024
dbd57e3
Fixed issue where TaintTracking was not catching matchAll vulnerability
Napalys Nov 7, 2024
c2baf0b
Added test where RegExp. is used after matchAll but it not flagged as…
Napalys Nov 8, 2024
70cf1a5
Now catches usage of RegExp. after matchAll usage.
Napalys Nov 8, 2024
1eabb6c
Update javascript/ql/test/experimental/Security/CWE-918/check-regex.js
Napalys Nov 11, 2024
00790bf
Added change notes
Napalys Nov 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion javascript/ql/lib/semmle/javascript/MembershipCandidates.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ module MembershipCandidate {
or
// u.match(/re/) or u.match("re")
base = this and
m = "match" and
m = ["match", "matchAll"] and
enumeration = RegExp::getRegExpFromNode(firstArg)
)
}
Expand Down
4 changes: 2 additions & 2 deletions javascript/ql/lib/semmle/javascript/Regexp.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -938,7 +938,7 @@ private predicate isMatchObjectProperty(string name) {

/** Holds if `call` is a call to `match` whose result is used in a way that is incompatible with Match objects. */
private predicate isUsedAsNonMatchObject(DataFlow::MethodCallNode call) {
call.getMethodName() = "match" and
call.getMethodName() = ["match", "matchAll"] and
call.getNumArgument() = 1 and
(
// Accessing a property that is absent on Match objects
Expand Down Expand Up @@ -996,7 +996,7 @@ predicate isInterpretedAsRegExp(DataFlow::Node source) {
not isNativeStringMethod(func, methodName)
)
|
methodName = "match" and
methodName = ["match", "matchAll"] and
source = mce.getArgument(0) and
mce.getNumArgument() = 1 and
not isUsedAsNonMatchObject(mce)
Expand Down
2 changes: 1 addition & 1 deletion javascript/ql/lib/semmle/javascript/StringOps.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -722,7 +722,7 @@ module StringOps {
}

private class MatchCall extends DataFlow::MethodCallNode {
MatchCall() { this.getMethodName() = "match" }
MatchCall() { this.getMethodName() = ["match", "matchAll"] }
}

private class ExecCall extends DataFlow::MethodCallNode {
Expand Down
6 changes: 3 additions & 3 deletions javascript/ql/lib/semmle/javascript/dataflow/TaintTracking.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -716,7 +716,7 @@ module TaintTracking {

pragma[nomagic]
private DataFlow::MethodCallNode matchMethodCall() {
result.getMethodName() = "match" and
result.getMethodName() = ["match", "matchAll"] and
exists(DataFlow::AnalyzedNode analyzed |
pragma[only_bind_into](analyzed) = result.getArgument(0).analyze() and
analyzed.getAType() = TTRegExp()
Expand Down Expand Up @@ -917,7 +917,7 @@ module TaintTracking {
*/
private ControlFlowNode getACaptureSetter(DataFlow::Node input) {
exists(DataFlow::MethodCallNode call | result = call.asExpr() |
call.getMethodName() = ["search", "replace", "replaceAll", "match"] and
call.getMethodName() = ["search", "replace", "replaceAll", "match", "matchAll"] and
input = call.getReceiver()
or
call.getMethodName() = ["test", "exec"] and input = call.getArgument(0)
Expand Down Expand Up @@ -998,7 +998,7 @@ module TaintTracking {
or
// u.match(/re/) or u.match("re")
base = expr and
m = "match" and
m = ["match", "matchAll"] and
RegExp::isGenericRegExpSanitizer(RegExp::getRegExpFromNode(firstArg.flow()),
sanitizedOutcome)
)
Expand Down
2 changes: 1 addition & 1 deletion javascript/ql/src/Security/CWE-020/MissingRegExpAnchor.ql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ private module Impl implements
|
name = "replace"
or
name = "match" and exists(mcn.getAPropertyRead())
name = ["match", "matchAll"] and exists(mcn.getAPropertyRead())
)
)
}
Expand Down
9 changes: 9 additions & 0 deletions javascript/ql/test/experimental/Security/CWE-918/SSRF.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,10 @@ nodes
| check-regex.js:41:13:41:43 | "test.c ... tainted |
| check-regex.js:41:27:41:43 | req.query.tainted |
| check-regex.js:41:27:41:43 | req.query.tainted |
| check-regex.js:61:15:61:42 | baseURL ... tainted |
| check-regex.js:61:15:61:42 | baseURL ... tainted |
| check-regex.js:61:25:61:42 | req.params.tainted |
| check-regex.js:61:25:61:42 | req.params.tainted |
| check-validator.js:15:15:15:45 | "test.c ... tainted |
| check-validator.js:15:15:15:45 | "test.c ... tainted |
| check-validator.js:15:29:15:45 | req.query.tainted |
Expand Down Expand Up @@ -127,6 +131,10 @@ edges
| check-regex.js:41:27:41:43 | req.query.tainted | check-regex.js:41:13:41:43 | "test.c ... tainted |
| check-regex.js:41:27:41:43 | req.query.tainted | check-regex.js:41:13:41:43 | "test.c ... tainted |
| check-regex.js:41:27:41:43 | req.query.tainted | check-regex.js:41:13:41:43 | "test.c ... tainted |
| check-regex.js:61:25:61:42 | req.params.tainted | check-regex.js:61:15:61:42 | baseURL ... tainted |
| check-regex.js:61:25:61:42 | req.params.tainted | check-regex.js:61:15:61:42 | baseURL ... tainted |
| check-regex.js:61:25:61:42 | req.params.tainted | check-regex.js:61:15:61:42 | baseURL ... tainted |
| check-regex.js:61:25:61:42 | req.params.tainted | check-regex.js:61:15:61:42 | baseURL ... tainted |
| check-validator.js:15:29:15:45 | req.query.tainted | check-validator.js:15:15:15:45 | "test.c ... tainted |
| check-validator.js:15:29:15:45 | req.query.tainted | check-validator.js:15:15:15:45 | "test.c ... tainted |
| check-validator.js:15:29:15:45 | req.query.tainted | check-validator.js:15:15:15:45 | "test.c ... tainted |
Expand Down Expand Up @@ -166,6 +174,7 @@ edges
| check-regex.js:31:15:31:45 | "test.c ... tainted | check-regex.js:31:29:31:45 | req.query.tainted | check-regex.js:31:15:31:45 | "test.c ... tainted | The URL of this request depends on a user-provided value. |
| check-regex.js:34:15:34:42 | baseURL ... tainted | check-regex.js:34:25:34:42 | req.params.tainted | check-regex.js:34:15:34:42 | baseURL ... tainted | The URL of this request depends on a user-provided value. |
| check-regex.js:41:13:41:43 | "test.c ... tainted | check-regex.js:41:27:41:43 | req.query.tainted | check-regex.js:41:13:41:43 | "test.c ... tainted | The URL of this request depends on a user-provided value. |
| check-regex.js:61:15:61:42 | baseURL ... tainted | check-regex.js:61:25:61:42 | req.params.tainted | check-regex.js:61:15:61:42 | baseURL ... tainted | The URL of this request depends on a user-provided value. |
| check-validator.js:15:15:15:45 | "test.c ... tainted | check-validator.js:15:29:15:45 | req.query.tainted | check-validator.js:15:15:15:45 | "test.c ... tainted | The URL of this request depends on a user-provided value. |
| check-validator.js:27:15:27:45 | "test.c ... tainted | check-validator.js:27:29:27:45 | req.query.tainted | check-validator.js:27:15:27:45 | "test.c ... tainted | The URL of this request depends on a user-provided value. |
| check-validator.js:50:15:50:45 | "test.c ... tainted | check-validator.js:50:29:50:45 | req.query.tainted | check-validator.js:50:15:50:45 | "test.c ... tainted | The URL of this request depends on a user-provided value. |
Expand Down
23 changes: 22 additions & 1 deletion javascript/ql/test/experimental/Security/CWE-918/check-regex.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ app.get('/check-with-axios', req => {
} else {
axios.get(baseURL + req.params.tainted); // OK
}

// Blacklists are not safe
if (!req.query.tainted.match(/^[/\.%]+$/)) {
axios.get("test.com/" + req.query.tainted); // SSRF
Expand All @@ -39,8 +39,29 @@ app.get('/check-with-axios', req => {
}

axios.get("test.com/" + req.query.tainted); // OK - False Positive

if (req.query.tainted.matchAll(/^[0-9a-z]+$/g)) { // letters and numbers
axios.get("test.com/" + req.query.tainted); // OK
}
if (req.query.tainted.matchAll(/^[0-9a-z\-_]+$/g)) { // letters, numbers, - and _
axios.get("test.com/" + req.query.tainted); // OK
}
});

const isValidPath = path => path.match(/^[0-9a-z]+$/);

const isInBlackList = path => path.match(/^[/\.%]+$/);

app.get('/check-with-axios', req => {
const baseURL = "test.com/"
if (isValidPathMatchAll(req.params.tainted) ) {
axios.get(baseURL + req.params.tainted); // OK
}
if (!isValidPathMatchAll(req.params.tainted) ) {
axios.get(baseURL + req.params.tainted); // SSRF
} else {
axios.get(baseURL + req.params.tainted); // OK
}
});

const isValidPathMatchAll = path => path.matchAll(/^[0-9a-z]+$/g);
1 change: 1 addition & 0 deletions ...t/query-tests/Security/CWE-020/IncompleteHostnameRegExp/IncompleteHostnameRegExp.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,4 @@
| tst-IncompleteHostnameRegExp.js:53:14:53:35 | test.example.com$ | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:53:13:53:36 | 'test.' ... e.com$' | here |
| tst-IncompleteHostnameRegExp.js:55:14:55:38 | ^http://test.example.com | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:55:13:55:39 | '^http: ... le.com' | here |
| tst-IncompleteHostnameRegExp.js:59:5:59:20 | foo.example\\.com | This regular expression has an unescaped '.' before 'example\\.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:59:2:59:32 | /^(foo. ... ever)$/ | here |
| tst-IncompleteHostnameRegExp.js:61:18:61:41 | ^http://test.example.com | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:61:17:61:42 | "^http: ... le.com" | here |
2 changes: 2 additions & 0 deletions ...est/query-tests/Security/CWE-020/IncompleteHostnameRegExp/tst-IncompleteHostnameRegExp.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,4 +57,6 @@
/^http:\/\/(..|...)\.example\.com\/index\.html/; // OK, wildcards are intentional
/^http:\/\/.\.example\.com\/index\.html/; // OK, the wildcard is intentional
/^(foo.example\.com|whatever)$/; // kinda OK - one disjunction doesn't even look like a hostname

if (s.matchAll("^http://test.example.com")) {} // NOT OK
});
9 changes: 9 additions & 0 deletions ...ipt/ql/test/query-tests/Security/CWE-020/MissingRegExpAnchor/MissingRegExpAnchor.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,3 +59,12 @@
| tst-UnanchoredUrlRegExp.js:26:3:26:22 | "^https?://good.com" | This hostname pattern may match any domain name, as it is missing a '$' or '/' at the end. |
| tst-UnanchoredUrlRegExp.js:35:2:35:32 | /https? ... 0-9]+)/ | When this is used as a regular expression on a URL, it may match anywhere, and arbitrary hosts may come before or after it. |
| tst-UnanchoredUrlRegExp.js:77:11:77:32 | /vimeo\\ ... 0-9]+)/ | When this is used as a regular expression on a URL, it may match anywhere, and arbitrary hosts may come before or after it. |
| tst-UnanchoredUrlRegExp.js:111:50:111:68 | "https?://good.com" | When this is used as a regular expression on a URL, it may match anywhere, and arbitrary hosts may come before or after it. |
| tst-UnanchoredUrlRegExp.js:112:61:112:79 | "https?://good.com" | When this is used as a regular expression on a URL, it may match anywhere, and arbitrary hosts may come before or after it. |
| tst-UnanchoredUrlRegExp.js:113:50:113:69 | "^https?://good.com" | This hostname pattern may match any domain name, as it is missing a '$' or '/' at the end. |
| tst-UnanchoredUrlRegExp.js:114:50:114:72 | /^https ... d.com/g | This hostname pattern may match any domain name, as it is missing a '$' or '/' at the end. |
| tst-UnanchoredUrlRegExp.js:115:50:115:94 | "(^http ... 2.com)" | This hostname pattern may match any domain name, as it is missing a '$' or '/' at the end. |
| tst-UnanchoredUrlRegExp.js:116:50:116:93 | "(https ... e.com)" | This hostname pattern may match any domain name, as it is missing a '$' or '/' at the end. |
| tst-UnanchoredUrlRegExp.js:117:50:117:59 | "good.com" | When this is used as a regular expression on a URL, it may match anywhere, and arbitrary hosts may come before or after it. |
| tst-UnanchoredUrlRegExp.js:118:50:118:68 | "https?://good.com" | When this is used as a regular expression on a URL, it may match anywhere, and arbitrary hosts may come before or after it. |
| tst-UnanchoredUrlRegExp.js:119:50:119:73 | "https? ... m:8080" | When this is used as a regular expression on a URL, it may match anywhere, and arbitrary hosts may come before or after it. |
23 changes: 23 additions & 0 deletions ...cript/ql/test/query-tests/Security/CWE-020/MissingRegExpAnchor/tst-UnanchoredUrlRegExp.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,4 +105,27 @@

/\.com|\.org/; // OK, has no domain name
/example\.com|whatever/; // OK, the other disjunction doesn't match a hostname

// MatchAll test cases:
// Vulnerable patterns
if ("http://evil.com/?http://good.com".matchAll("https?://good.com")) {} // NOT OK
if ("http://evil.com/?http://good.com".matchAll(new RegExp("https?://good.com"))) {} // NOT OK
if ("http://evil.com/?http://good.com".matchAll("^https?://good.com")) {} // NOT OK - missing post-anchor
if ("http://evil.com/?http://good.com".matchAll(/^https?:\/\/good.com/g)) {} // NOT OK - missing post-anchor
if ("http://evil.com/?http://good.com".matchAll("(^https?://good1.com)|(^https?://good2.com)")) {} // NOT OK - missing post-anchor
if ("http://evil.com/?http://good.com".matchAll("(https?://good.com)|(^https?://goodie.com)")) {} // NOT OK - missing post-anchor
if ("http://evil.com/?http://good.com".matchAll("good.com")) {} // NOT OK - missing protocol
if ("http://evil.com/?http://good.com".matchAll("https?://good.com")) {} // NOT OK
if ("http://evil.com/?http://good.com".matchAll("https?://good.com:8080")) {} // NOT OK

// Non-vulnerable patterns
if ("something".matchAll("other")) {} // OK
if ("something".matchAll("x.commissary")) {} // OK
if ("http://evil.com/?http://good.com".matchAll("^https?://good.com$")) {} // OK
if ("http://evil.com/?http://good.com".matchAll(new RegExp("^https?://good.com$"))) {} // OK
if ("http://evil.com/?http://good.com".matchAll("^https?://good.com/$")) {} // OK
if ("http://evil.com/?http://good.com".matchAll(/^https?:\/\/good.com\/$/)) {} // OK
if ("http://evil.com/?http://good.com".matchAll("(^https?://good1.com$)|(^https?://good2.com$)")) {} // OK
if ("http://evil.com/?http://good.com".matchAll("(https?://good.com$)|(^https?://goodie.com$)")) {} // OK

});
29 changes: 29 additions & 0 deletions javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/TaintedPath.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2237,6 +2237,19 @@ nodes
| normalizedPaths.js:408:38:408:59 | req.que ... it('/') |
| normalizedPaths.js:408:38:408:59 | req.que ... it('/') |
| normalizedPaths.js:408:38:408:59 | req.que ... it('/') |
| normalizedPaths.js:412:7:412:46 | path |
| normalizedPaths.js:412:7:412:46 | path |
| normalizedPaths.js:412:14:412:46 | pathMod ... uery.x) |
| normalizedPaths.js:412:14:412:46 | pathMod ... uery.x) |
| normalizedPaths.js:412:35:412:45 | req.query.x |
| normalizedPaths.js:412:35:412:45 | req.query.x |
| normalizedPaths.js:412:35:412:45 | req.query.x |
| normalizedPaths.js:415:19:415:22 | path |
| normalizedPaths.js:415:19:415:22 | path |
| normalizedPaths.js:415:19:415:22 | path |
| normalizedPaths.js:426:21:426:24 | path |
| normalizedPaths.js:426:21:426:24 | path |
| normalizedPaths.js:426:21:426:24 | path |
| other-fs-libraries.js:9:7:9:48 | path |
| other-fs-libraries.js:9:7:9:48 | path |
| other-fs-libraries.js:9:7:9:48 | path |
Expand Down Expand Up @@ -7524,6 +7537,20 @@ edges
| normalizedPaths.js:408:38:408:59 | req.que ... it('/') | normalizedPaths.js:408:19:408:60 | pathMod ... t('/')) |
| normalizedPaths.js:408:38:408:59 | req.que ... it('/') | normalizedPaths.js:408:19:408:60 | pathMod ... t('/')) |
| normalizedPaths.js:408:38:408:59 | req.que ... it('/') | normalizedPaths.js:408:19:408:60 | pathMod ... t('/')) |
| normalizedPaths.js:412:7:412:46 | path | normalizedPaths.js:415:19:415:22 | path |
| normalizedPaths.js:412:7:412:46 | path | normalizedPaths.js:415:19:415:22 | path |
| normalizedPaths.js:412:7:412:46 | path | normalizedPaths.js:415:19:415:22 | path |
| normalizedPaths.js:412:7:412:46 | path | normalizedPaths.js:415:19:415:22 | path |
| normalizedPaths.js:412:7:412:46 | path | normalizedPaths.js:426:21:426:24 | path |
| normalizedPaths.js:412:7:412:46 | path | normalizedPaths.js:426:21:426:24 | path |
| normalizedPaths.js:412:7:412:46 | path | normalizedPaths.js:426:21:426:24 | path |
| normalizedPaths.js:412:7:412:46 | path | normalizedPaths.js:426:21:426:24 | path |
| normalizedPaths.js:412:14:412:46 | pathMod ... uery.x) | normalizedPaths.js:412:7:412:46 | path |
| normalizedPaths.js:412:14:412:46 | pathMod ... uery.x) | normalizedPaths.js:412:7:412:46 | path |
| normalizedPaths.js:412:35:412:45 | req.query.x | normalizedPaths.js:412:14:412:46 | pathMod ... uery.x) |
| normalizedPaths.js:412:35:412:45 | req.query.x | normalizedPaths.js:412:14:412:46 | pathMod ... uery.x) |
| normalizedPaths.js:412:35:412:45 | req.query.x | normalizedPaths.js:412:14:412:46 | pathMod ... uery.x) |
| normalizedPaths.js:412:35:412:45 | req.query.x | normalizedPaths.js:412:14:412:46 | pathMod ... uery.x) |
| other-fs-libraries.js:9:7:9:48 | path | other-fs-libraries.js:11:19:11:22 | path |
| other-fs-libraries.js:9:7:9:48 | path | other-fs-libraries.js:11:19:11:22 | path |
| other-fs-libraries.js:9:7:9:48 | path | other-fs-libraries.js:11:19:11:22 | path |
Expand Down Expand Up @@ -10539,6 +10566,8 @@ edges
| normalizedPaths.js:399:21:399:24 | path | normalizedPaths.js:385:35:385:45 | req.query.x | normalizedPaths.js:399:21:399:24 | path | This path depends on a $@. | normalizedPaths.js:385:35:385:45 | req.query.x | user-provided value |
| normalizedPaths.js:407:19:407:67 | pathMod ... t('/')) | normalizedPaths.js:407:45:407:55 | req.query.x | normalizedPaths.js:407:19:407:67 | pathMod ... t('/')) | This path depends on a $@. | normalizedPaths.js:407:45:407:55 | req.query.x | user-provided value |
| normalizedPaths.js:408:19:408:60 | pathMod ... t('/')) | normalizedPaths.js:408:38:408:48 | req.query.x | normalizedPaths.js:408:19:408:60 | pathMod ... t('/')) | This path depends on a $@. | normalizedPaths.js:408:38:408:48 | req.query.x | user-provided value |
| normalizedPaths.js:415:19:415:22 | path | normalizedPaths.js:412:35:412:45 | req.query.x | normalizedPaths.js:415:19:415:22 | path | This path depends on a $@. | normalizedPaths.js:412:35:412:45 | req.query.x | user-provided value |
| normalizedPaths.js:426:21:426:24 | path | normalizedPaths.js:412:35:412:45 | req.query.x | normalizedPaths.js:426:21:426:24 | path | This path depends on a $@. | normalizedPaths.js:412:35:412:45 | req.query.x | user-provided value |
| other-fs-libraries.js:11:19:11:22 | path | other-fs-libraries.js:9:24:9:30 | req.url | other-fs-libraries.js:11:19:11:22 | path | This path depends on a $@. | other-fs-libraries.js:9:24:9:30 | req.url | user-provided value |
| other-fs-libraries.js:12:27:12:30 | path | other-fs-libraries.js:9:24:9:30 | req.url | other-fs-libraries.js:12:27:12:30 | path | This path depends on a $@. | other-fs-libraries.js:9:24:9:30 | req.url | user-provided value |
| other-fs-libraries.js:13:24:13:27 | path | other-fs-libraries.js:9:24:9:30 | req.url | other-fs-libraries.js:13:24:13:27 | path | This path depends on a $@. | other-fs-libraries.js:9:24:9:30 | req.url | user-provided value |
Expand Down
22 changes: 22 additions & 0 deletions javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/normalizedPaths.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -407,3 +407,25 @@ app.get('/join-spread', (req, res) => {
fs.readFileSync(pathModule.join('foo', ...req.query.x.split('/'))); // NOT OK
fs.readFileSync(pathModule.join(...req.query.x.split('/'))); // NOT OK
});

app.get('/dotdot-matchAll-regexp', (req, res) => {
let path = pathModule.normalize(req.query.x);
if (pathModule.isAbsolute(path))
return;
fs.readFileSync(path); // NOT OK
if (!path.matchAll(/\./)) {
fs.readFileSync(path); // OK
}
if (!path.matchAll(/\.\./)) {
fs.readFileSync(path); // OK
}
if (!path.matchAll(/\.\.\//)) {
fs.readFileSync(path); // OK
}
if (!path.matchAll(/\.\.\/foo/)) {
fs.readFileSync(path); // NOT OK
}
if (!path.matchAll(/(\.\.\/|\.\.\\)/)) {
fs.readFileSync(path); // OK
}
});
Loading