From 584b4f51aa8706147fe18437545fc735d5843056 Mon Sep 17 00:00:00 2001 From: Napalys Klicius Date: Fri, 20 Jun 2025 16:26:59 +0200 Subject: [PATCH 1/3] JS: add false positive test cases for hostname regex detection --- .../IncompleteHostnameRegExp.expected | 1 + .../tst-IncompleteHostnameRegExp.js | 4 ++++ .../MissingRegExpAnchor/MissingRegExpAnchor.expected | 1 + .../query-tests/Security/CWE-020/MissingRegExpAnchor/tst.js | 6 ++++++ 4 files changed, 12 insertions(+) create mode 100644 javascript/ql/test/query-tests/Security/CWE-020/MissingRegExpAnchor/tst.js diff --git a/javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp/IncompleteHostnameRegExp.expected b/javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp/IncompleteHostnameRegExp.expected index 90d4e925d21e..af7c020ce3b9 100644 --- a/javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp/IncompleteHostnameRegExp.expected +++ b/javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp/IncompleteHostnameRegExp.expected @@ -27,3 +27,4 @@ | tst-IncompleteHostnameRegExp.js:56:14:56: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:56:13:56:39 | '^http: ... le.com' | here | | tst-IncompleteHostnameRegExp.js:60:5:60:20 | foo.example\\.com | This regular expression has an unescaped '.' before 'example\\.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:60:2:60:32 | /^(foo. ... ever)$/ | here | | tst-IncompleteHostnameRegExp.js:62:18:62: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:62:17:62:42 | "^http: ... le.com" | here | +| tst-IncompleteHostnameRegExp.js:65:24:65:38 | https://a.b.com | This string, which is used as a regular expression $@, has an unescaped '.' before 'b.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:66:58:66:69 | megacliteUrl | here | diff --git a/javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp/tst-IncompleteHostnameRegExp.js b/javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp/tst-IncompleteHostnameRegExp.js index ae0447f132fd..5e16491a01e4 100644 --- a/javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp/tst-IncompleteHostnameRegExp.js +++ b/javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp/tst-IncompleteHostnameRegExp.js @@ -60,4 +60,8 @@ /^(foo.example\.com|whatever)$/; // $ Alert (but kinda OK - one disjunction doesn't even look like a hostname) if (s.matchAll("^http://test.example.com")) {} // $ Alert + + const sinon = require('sinon'); + const megacliteUrl = "https://a.b.com"; // $SPURIOUS:Alert + sinon.assert.calledWith(postStub.firstCall, sinon.match(megacliteUrl)); }); diff --git a/javascript/ql/test/query-tests/Security/CWE-020/MissingRegExpAnchor/MissingRegExpAnchor.expected b/javascript/ql/test/query-tests/Security/CWE-020/MissingRegExpAnchor/MissingRegExpAnchor.expected index ebff0b583c02..d29b0411a491 100644 --- a/javascript/ql/test/query-tests/Security/CWE-020/MissingRegExpAnchor/MissingRegExpAnchor.expected +++ b/javascript/ql/test/query-tests/Security/CWE-020/MissingRegExpAnchor/MissingRegExpAnchor.expected @@ -68,3 +68,4 @@ | 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. | +| tst.js:4:24:4:40 | "https://a.b.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. | diff --git a/javascript/ql/test/query-tests/Security/CWE-020/MissingRegExpAnchor/tst.js b/javascript/ql/test/query-tests/Security/CWE-020/MissingRegExpAnchor/tst.js new file mode 100644 index 000000000000..1214e5a301d4 --- /dev/null +++ b/javascript/ql/test/query-tests/Security/CWE-020/MissingRegExpAnchor/tst.js @@ -0,0 +1,6 @@ +const sinon = require('sinon'); + +function testFunction() { + const megacliteUrl = "https://a.b.com"; // $SPURIOUS:Alert + sinon.assert.calledWith(postStub.firstCall, sinon.match(megacliteUrl)); +} From ef51ab172f6cb6ba549c4196f81b8ed4578dcc24 Mon Sep 17 00:00:00 2001 From: Napalys Klicius Date: Fri, 20 Jun 2025 16:28:22 +0200 Subject: [PATCH 2/3] JS: exclude `sinon` module from regexp match calls --- javascript/ql/lib/semmle/javascript/Regexp.qll | 2 ++ .../IncompleteHostnameRegExp/IncompleteHostnameRegExp.expected | 1 - .../IncompleteHostnameRegExp/tst-IncompleteHostnameRegExp.js | 2 +- .../CWE-020/MissingRegExpAnchor/MissingRegExpAnchor.expected | 1 - .../query-tests/Security/CWE-020/MissingRegExpAnchor/tst.js | 2 +- 5 files changed, 4 insertions(+), 4 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/Regexp.qll b/javascript/ql/lib/semmle/javascript/Regexp.qll index 642a3d196fb7..ea2993ae7da8 100644 --- a/javascript/ql/lib/semmle/javascript/Regexp.qll +++ b/javascript/ql/lib/semmle/javascript/Regexp.qll @@ -998,6 +998,8 @@ private predicate isUsedAsNonMatchObject(DataFlow::MethodCallNode call) { or // Result is obviously unused call.asExpr() = any(ExprStmt stmt).getExpr() + or + call = API::moduleImport("sinon").getMember("match").getACall() ) } diff --git a/javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp/IncompleteHostnameRegExp.expected b/javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp/IncompleteHostnameRegExp.expected index af7c020ce3b9..90d4e925d21e 100644 --- a/javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp/IncompleteHostnameRegExp.expected +++ b/javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp/IncompleteHostnameRegExp.expected @@ -27,4 +27,3 @@ | tst-IncompleteHostnameRegExp.js:56:14:56: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:56:13:56:39 | '^http: ... le.com' | here | | tst-IncompleteHostnameRegExp.js:60:5:60:20 | foo.example\\.com | This regular expression has an unescaped '.' before 'example\\.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:60:2:60:32 | /^(foo. ... ever)$/ | here | | tst-IncompleteHostnameRegExp.js:62:18:62: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:62:17:62:42 | "^http: ... le.com" | here | -| tst-IncompleteHostnameRegExp.js:65:24:65:38 | https://a.b.com | This string, which is used as a regular expression $@, has an unescaped '.' before 'b.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:66:58:66:69 | megacliteUrl | here | diff --git a/javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp/tst-IncompleteHostnameRegExp.js b/javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp/tst-IncompleteHostnameRegExp.js index 5e16491a01e4..b2e030a54921 100644 --- a/javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp/tst-IncompleteHostnameRegExp.js +++ b/javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp/tst-IncompleteHostnameRegExp.js @@ -62,6 +62,6 @@ if (s.matchAll("^http://test.example.com")) {} // $ Alert const sinon = require('sinon'); - const megacliteUrl = "https://a.b.com"; // $SPURIOUS:Alert + const megacliteUrl = "https://a.b.com"; sinon.assert.calledWith(postStub.firstCall, sinon.match(megacliteUrl)); }); diff --git a/javascript/ql/test/query-tests/Security/CWE-020/MissingRegExpAnchor/MissingRegExpAnchor.expected b/javascript/ql/test/query-tests/Security/CWE-020/MissingRegExpAnchor/MissingRegExpAnchor.expected index d29b0411a491..ebff0b583c02 100644 --- a/javascript/ql/test/query-tests/Security/CWE-020/MissingRegExpAnchor/MissingRegExpAnchor.expected +++ b/javascript/ql/test/query-tests/Security/CWE-020/MissingRegExpAnchor/MissingRegExpAnchor.expected @@ -68,4 +68,3 @@ | 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. | -| tst.js:4:24:4:40 | "https://a.b.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. | diff --git a/javascript/ql/test/query-tests/Security/CWE-020/MissingRegExpAnchor/tst.js b/javascript/ql/test/query-tests/Security/CWE-020/MissingRegExpAnchor/tst.js index 1214e5a301d4..d4e4d97b651e 100644 --- a/javascript/ql/test/query-tests/Security/CWE-020/MissingRegExpAnchor/tst.js +++ b/javascript/ql/test/query-tests/Security/CWE-020/MissingRegExpAnchor/tst.js @@ -1,6 +1,6 @@ const sinon = require('sinon'); function testFunction() { - const megacliteUrl = "https://a.b.com"; // $SPURIOUS:Alert + const megacliteUrl = "https://a.b.com"; sinon.assert.calledWith(postStub.firstCall, sinon.match(megacliteUrl)); } From 33f42444d555ecf8add9657ae2fd61c73f72ded0 Mon Sep 17 00:00:00 2001 From: Napalys Klicius Date: Fri, 20 Jun 2025 16:31:18 +0200 Subject: [PATCH 3/3] JS: add change note --- javascript/ql/lib/change-notes/2025-06-20-sinon.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 javascript/ql/lib/change-notes/2025-06-20-sinon.md diff --git a/javascript/ql/lib/change-notes/2025-06-20-sinon.md b/javascript/ql/lib/change-notes/2025-06-20-sinon.md new file mode 100644 index 000000000000..fd8b8e0ad079 --- /dev/null +++ b/javascript/ql/lib/change-notes/2025-06-20-sinon.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* Calls to `sinon.match()` are no longer incorrectly identified as regular expression operations.