hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-26 01:05:15 +02:00
Code Issues Packages Projects Releases Wiki Activity

Added support for matchAll in CWE-020 including new test cases

Browse Source
This commit is contained in:
Napalys
2024-11-05 08:51:24 +01:00
parent 7b870d30a4
commit ccee34d6d3
5 changed files with 37 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
javascript/ql/lib/semmle/javascript/Regexp.qll
View File

@@ -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
@@ -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)

2
javascript/ql/lib/semmle/javascript/StringOps.qll
View File

@@ -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 {

2
javascript/ql/src/Security/CWE-020/MissingRegExpAnchor.ql
View File

@@ -36,6 +36,8 @@ private module Impl implements
name = "replace"
or
name = "match" and exists(mcn.getAPropertyRead())
or
name = "matchAll" and exists(mcn.getAPropertyRead())
)
)
}

9
javascript/ql/test/query-tests/Security/CWE-020/MissingRegExpAnchor/MissingRegExpAnchor.expected
View File

@@ -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
javascript/ql/test/query-tests/Security/CWE-020/MissingRegExpAnchor/tst-UnanchoredUrlRegExp.js
View File

@@ -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
});