From 18532bae54eacb812b86e8c4541ce2ff553e6c6e Mon Sep 17 00:00:00 2001 From: Erik Krogh Kristensen Date: Fri, 8 Apr 2022 10:28:30 +0200 Subject: [PATCH 01/11] move js/missing-postmessageorigin-verification out of experimental --- .../Security/CWE-020/PostMessageNoOriginCheck.qhelp | 0 .../Security/CWE-020/PostMessageNoOriginCheck.ql | 0 .../Security/CWE-020/examples/postMessageInsufficientCheck.js | 0 .../Security/CWE-020/examples/postMessageNoOriginCheck.js | 0 .../Security/CWE-020/examples/postMessageWithOriginCheck.js | 0 5 files changed, 0 insertions(+), 0 deletions(-) rename javascript/ql/src/{experimental => }/Security/CWE-020/PostMessageNoOriginCheck.qhelp (100%) rename javascript/ql/src/{experimental => }/Security/CWE-020/PostMessageNoOriginCheck.ql (100%) rename javascript/ql/src/{experimental => }/Security/CWE-020/examples/postMessageInsufficientCheck.js (100%) rename javascript/ql/src/{experimental => }/Security/CWE-020/examples/postMessageNoOriginCheck.js (100%) rename javascript/ql/src/{experimental => }/Security/CWE-020/examples/postMessageWithOriginCheck.js (100%) diff --git a/javascript/ql/src/experimental/Security/CWE-020/PostMessageNoOriginCheck.qhelp b/javascript/ql/src/Security/CWE-020/PostMessageNoOriginCheck.qhelp similarity index 100% rename from javascript/ql/src/experimental/Security/CWE-020/PostMessageNoOriginCheck.qhelp rename to javascript/ql/src/Security/CWE-020/PostMessageNoOriginCheck.qhelp diff --git a/javascript/ql/src/experimental/Security/CWE-020/PostMessageNoOriginCheck.ql b/javascript/ql/src/Security/CWE-020/PostMessageNoOriginCheck.ql similarity index 100% rename from javascript/ql/src/experimental/Security/CWE-020/PostMessageNoOriginCheck.ql rename to javascript/ql/src/Security/CWE-020/PostMessageNoOriginCheck.ql diff --git a/javascript/ql/src/experimental/Security/CWE-020/examples/postMessageInsufficientCheck.js b/javascript/ql/src/Security/CWE-020/examples/postMessageInsufficientCheck.js similarity index 100% rename from javascript/ql/src/experimental/Security/CWE-020/examples/postMessageInsufficientCheck.js rename to javascript/ql/src/Security/CWE-020/examples/postMessageInsufficientCheck.js diff --git a/javascript/ql/src/experimental/Security/CWE-020/examples/postMessageNoOriginCheck.js b/javascript/ql/src/Security/CWE-020/examples/postMessageNoOriginCheck.js similarity index 100% rename from javascript/ql/src/experimental/Security/CWE-020/examples/postMessageNoOriginCheck.js rename to javascript/ql/src/Security/CWE-020/examples/postMessageNoOriginCheck.js diff --git a/javascript/ql/src/experimental/Security/CWE-020/examples/postMessageWithOriginCheck.js b/javascript/ql/src/Security/CWE-020/examples/postMessageWithOriginCheck.js similarity index 100% rename from javascript/ql/src/experimental/Security/CWE-020/examples/postMessageWithOriginCheck.js rename to javascript/ql/src/Security/CWE-020/examples/postMessageWithOriginCheck.js From ec9c308d0603a2a159fdbbc5bb7d9b0175b7652f Mon Sep 17 00:00:00 2001 From: Erik Krogh Kristensen Date: Fri, 8 Apr 2022 10:58:40 +0200 Subject: [PATCH 02/11] reorganize the tests in CWE-020 --- .../IncompleteHostnameRegExp.expected | 0 .../{ => IncompleteHostnameRegExp}/IncompleteHostnameRegExp.qlref | 0 .../tst-IncompleteHostnameRegExp.js | 0 .../IncompleteUrlSchemeCheck.expected | 0 .../{ => IncompleteUrlSchemeCheck}/IncompleteUrlSchemeCheck.js | 0 .../{ => IncompleteUrlSchemeCheck}/IncompleteUrlSchemeCheck.qlref | 0 .../IncompleteUrlSchemeCheckGood.js | 0 .../IncompleteUrlSubstringSanitization.expected | 0 .../IncompleteUrlSubstringSanitization.qlref | 0 .../tst-IncompleteUrlSubstringSanitization.js | 0 .../{ => IncorrectSuffixCheck}/IncorrectSuffixCheck.expected | 0 .../CWE-020/{ => IncorrectSuffixCheck}/IncorrectSuffixCheck.qlref | 0 .../{ => IncorrectSuffixCheck}/examples/IncorrectSuffixCheck.js | 0 .../examples/IncorrectSuffixCheckGood.js | 0 .../Security/CWE-020/{ => IncorrectSuffixCheck}/tst.js | 0 .../{ => MissingRegExpAnchor}/MissingRegExpAnchor.expected | 0 .../CWE-020/{ => MissingRegExpAnchor}/MissingRegExpAnchor.qlref | 0 .../CWE-020/{ => MissingRegExpAnchor}/tst-SemiAnchoredRegExp.js | 0 .../CWE-020/{ => MissingRegExpAnchor}/tst-UnanchoredUrlRegExp.js | 0 .../UntrustedDataToExternalAPI.expected | 0 .../UntrustedDataToExternalAPI.qlref | 0 .../tst-UntrustedDataToExternalAPI.js | 0 .../{ => UselessCharacterEscape}/UselessCharacterEscape.expected | 0 .../{ => UselessCharacterEscape}/UselessCharacterEscape.ql | 0 .../UselessRegExpCharacterEscape.expected | 0 .../UselessRegExpCharacterEscape.qlref | 0 .../Security/CWE-020/{ => UselessCharacterEscape}/tst-escapes.js | 0 27 files changed, 0 insertions(+), 0 deletions(-) rename javascript/ql/test/query-tests/Security/CWE-020/{ => IncompleteHostnameRegExp}/IncompleteHostnameRegExp.expected (100%) rename javascript/ql/test/query-tests/Security/CWE-020/{ => IncompleteHostnameRegExp}/IncompleteHostnameRegExp.qlref (100%) rename javascript/ql/test/query-tests/Security/CWE-020/{ => IncompleteHostnameRegExp}/tst-IncompleteHostnameRegExp.js (100%) rename javascript/ql/test/query-tests/Security/CWE-020/{ => IncompleteUrlSchemeCheck}/IncompleteUrlSchemeCheck.expected (100%) rename javascript/ql/test/query-tests/Security/CWE-020/{ => IncompleteUrlSchemeCheck}/IncompleteUrlSchemeCheck.js (100%) rename javascript/ql/test/query-tests/Security/CWE-020/{ => IncompleteUrlSchemeCheck}/IncompleteUrlSchemeCheck.qlref (100%) rename javascript/ql/test/query-tests/Security/CWE-020/{ => IncompleteUrlSchemeCheck}/IncompleteUrlSchemeCheckGood.js (100%) rename javascript/ql/test/query-tests/Security/CWE-020/{ => IncompleteUrlSubstringSanitization}/IncompleteUrlSubstringSanitization.expected (100%) rename javascript/ql/test/query-tests/Security/CWE-020/{ => IncompleteUrlSubstringSanitization}/IncompleteUrlSubstringSanitization.qlref (100%) rename javascript/ql/test/query-tests/Security/CWE-020/{ => IncompleteUrlSubstringSanitization}/tst-IncompleteUrlSubstringSanitization.js (100%) rename javascript/ql/test/query-tests/Security/CWE-020/{ => IncorrectSuffixCheck}/IncorrectSuffixCheck.expected (100%) rename javascript/ql/test/query-tests/Security/CWE-020/{ => IncorrectSuffixCheck}/IncorrectSuffixCheck.qlref (100%) rename javascript/ql/test/query-tests/Security/CWE-020/{ => IncorrectSuffixCheck}/examples/IncorrectSuffixCheck.js (100%) rename javascript/ql/test/query-tests/Security/CWE-020/{ => IncorrectSuffixCheck}/examples/IncorrectSuffixCheckGood.js (100%) rename javascript/ql/test/query-tests/Security/CWE-020/{ => IncorrectSuffixCheck}/tst.js (100%) rename javascript/ql/test/query-tests/Security/CWE-020/{ => MissingRegExpAnchor}/MissingRegExpAnchor.expected (100%) rename javascript/ql/test/query-tests/Security/CWE-020/{ => MissingRegExpAnchor}/MissingRegExpAnchor.qlref (100%) rename javascript/ql/test/query-tests/Security/CWE-020/{ => MissingRegExpAnchor}/tst-SemiAnchoredRegExp.js (100%) rename javascript/ql/test/query-tests/Security/CWE-020/{ => MissingRegExpAnchor}/tst-UnanchoredUrlRegExp.js (100%) rename javascript/ql/test/query-tests/Security/CWE-020/{ => UntrustedDataToExternalAPI}/UntrustedDataToExternalAPI.expected (100%) rename javascript/ql/test/query-tests/Security/CWE-020/{ => UntrustedDataToExternalAPI}/UntrustedDataToExternalAPI.qlref (100%) rename javascript/ql/test/query-tests/Security/CWE-020/{ => UntrustedDataToExternalAPI}/tst-UntrustedDataToExternalAPI.js (100%) rename javascript/ql/test/query-tests/Security/CWE-020/{ => UselessCharacterEscape}/UselessCharacterEscape.expected (100%) rename javascript/ql/test/query-tests/Security/CWE-020/{ => UselessCharacterEscape}/UselessCharacterEscape.ql (100%) rename javascript/ql/test/query-tests/Security/CWE-020/{ => UselessCharacterEscape}/UselessRegExpCharacterEscape.expected (100%) rename javascript/ql/test/query-tests/Security/CWE-020/{ => UselessCharacterEscape}/UselessRegExpCharacterEscape.qlref (100%) rename javascript/ql/test/query-tests/Security/CWE-020/{ => UselessCharacterEscape}/tst-escapes.js (100%) diff --git a/javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp.expected b/javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp/IncompleteHostnameRegExp.expected similarity index 100% rename from javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp.expected rename to javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp/IncompleteHostnameRegExp.expected diff --git a/javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp.qlref b/javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp/IncompleteHostnameRegExp.qlref similarity index 100% rename from javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp.qlref rename to javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp/IncompleteHostnameRegExp.qlref diff --git a/javascript/ql/test/query-tests/Security/CWE-020/tst-IncompleteHostnameRegExp.js b/javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp/tst-IncompleteHostnameRegExp.js similarity index 100% rename from javascript/ql/test/query-tests/Security/CWE-020/tst-IncompleteHostnameRegExp.js rename to javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp/tst-IncompleteHostnameRegExp.js diff --git a/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSchemeCheck.expected b/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSchemeCheck/IncompleteUrlSchemeCheck.expected similarity index 100% rename from javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSchemeCheck.expected rename to javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSchemeCheck/IncompleteUrlSchemeCheck.expected diff --git a/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSchemeCheck.js b/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSchemeCheck/IncompleteUrlSchemeCheck.js similarity index 100% rename from javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSchemeCheck.js rename to javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSchemeCheck/IncompleteUrlSchemeCheck.js diff --git a/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSchemeCheck.qlref b/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSchemeCheck/IncompleteUrlSchemeCheck.qlref similarity index 100% rename from javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSchemeCheck.qlref rename to javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSchemeCheck/IncompleteUrlSchemeCheck.qlref diff --git a/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSchemeCheckGood.js b/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSchemeCheck/IncompleteUrlSchemeCheckGood.js similarity index 100% rename from javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSchemeCheckGood.js rename to javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSchemeCheck/IncompleteUrlSchemeCheckGood.js diff --git a/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSubstringSanitization.expected b/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSubstringSanitization/IncompleteUrlSubstringSanitization.expected similarity index 100% rename from javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSubstringSanitization.expected rename to javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSubstringSanitization/IncompleteUrlSubstringSanitization.expected diff --git a/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSubstringSanitization.qlref b/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSubstringSanitization/IncompleteUrlSubstringSanitization.qlref similarity index 100% rename from javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSubstringSanitization.qlref rename to javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSubstringSanitization/IncompleteUrlSubstringSanitization.qlref diff --git a/javascript/ql/test/query-tests/Security/CWE-020/tst-IncompleteUrlSubstringSanitization.js b/javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSubstringSanitization/tst-IncompleteUrlSubstringSanitization.js similarity index 100% rename from javascript/ql/test/query-tests/Security/CWE-020/tst-IncompleteUrlSubstringSanitization.js rename to javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSubstringSanitization/tst-IncompleteUrlSubstringSanitization.js diff --git a/javascript/ql/test/query-tests/Security/CWE-020/IncorrectSuffixCheck.expected b/javascript/ql/test/query-tests/Security/CWE-020/IncorrectSuffixCheck/IncorrectSuffixCheck.expected similarity index 100% rename from javascript/ql/test/query-tests/Security/CWE-020/IncorrectSuffixCheck.expected rename to javascript/ql/test/query-tests/Security/CWE-020/IncorrectSuffixCheck/IncorrectSuffixCheck.expected diff --git a/javascript/ql/test/query-tests/Security/CWE-020/IncorrectSuffixCheck.qlref b/javascript/ql/test/query-tests/Security/CWE-020/IncorrectSuffixCheck/IncorrectSuffixCheck.qlref similarity index 100% rename from javascript/ql/test/query-tests/Security/CWE-020/IncorrectSuffixCheck.qlref rename to javascript/ql/test/query-tests/Security/CWE-020/IncorrectSuffixCheck/IncorrectSuffixCheck.qlref diff --git a/javascript/ql/test/query-tests/Security/CWE-020/examples/IncorrectSuffixCheck.js b/javascript/ql/test/query-tests/Security/CWE-020/IncorrectSuffixCheck/examples/IncorrectSuffixCheck.js similarity index 100% rename from javascript/ql/test/query-tests/Security/CWE-020/examples/IncorrectSuffixCheck.js rename to javascript/ql/test/query-tests/Security/CWE-020/IncorrectSuffixCheck/examples/IncorrectSuffixCheck.js diff --git a/javascript/ql/test/query-tests/Security/CWE-020/examples/IncorrectSuffixCheckGood.js b/javascript/ql/test/query-tests/Security/CWE-020/IncorrectSuffixCheck/examples/IncorrectSuffixCheckGood.js similarity index 100% rename from javascript/ql/test/query-tests/Security/CWE-020/examples/IncorrectSuffixCheckGood.js rename to javascript/ql/test/query-tests/Security/CWE-020/IncorrectSuffixCheck/examples/IncorrectSuffixCheckGood.js diff --git a/javascript/ql/test/query-tests/Security/CWE-020/tst.js b/javascript/ql/test/query-tests/Security/CWE-020/IncorrectSuffixCheck/tst.js similarity index 100% rename from javascript/ql/test/query-tests/Security/CWE-020/tst.js rename to javascript/ql/test/query-tests/Security/CWE-020/IncorrectSuffixCheck/tst.js diff --git a/javascript/ql/test/query-tests/Security/CWE-020/MissingRegExpAnchor.expected b/javascript/ql/test/query-tests/Security/CWE-020/MissingRegExpAnchor/MissingRegExpAnchor.expected similarity index 100% rename from javascript/ql/test/query-tests/Security/CWE-020/MissingRegExpAnchor.expected rename to javascript/ql/test/query-tests/Security/CWE-020/MissingRegExpAnchor/MissingRegExpAnchor.expected diff --git a/javascript/ql/test/query-tests/Security/CWE-020/MissingRegExpAnchor.qlref b/javascript/ql/test/query-tests/Security/CWE-020/MissingRegExpAnchor/MissingRegExpAnchor.qlref similarity index 100% rename from javascript/ql/test/query-tests/Security/CWE-020/MissingRegExpAnchor.qlref rename to javascript/ql/test/query-tests/Security/CWE-020/MissingRegExpAnchor/MissingRegExpAnchor.qlref diff --git a/javascript/ql/test/query-tests/Security/CWE-020/tst-SemiAnchoredRegExp.js b/javascript/ql/test/query-tests/Security/CWE-020/MissingRegExpAnchor/tst-SemiAnchoredRegExp.js similarity index 100% rename from javascript/ql/test/query-tests/Security/CWE-020/tst-SemiAnchoredRegExp.js rename to javascript/ql/test/query-tests/Security/CWE-020/MissingRegExpAnchor/tst-SemiAnchoredRegExp.js diff --git a/javascript/ql/test/query-tests/Security/CWE-020/tst-UnanchoredUrlRegExp.js b/javascript/ql/test/query-tests/Security/CWE-020/MissingRegExpAnchor/tst-UnanchoredUrlRegExp.js similarity index 100% rename from javascript/ql/test/query-tests/Security/CWE-020/tst-UnanchoredUrlRegExp.js rename to javascript/ql/test/query-tests/Security/CWE-020/MissingRegExpAnchor/tst-UnanchoredUrlRegExp.js diff --git a/javascript/ql/test/query-tests/Security/CWE-020/UntrustedDataToExternalAPI.expected b/javascript/ql/test/query-tests/Security/CWE-020/UntrustedDataToExternalAPI/UntrustedDataToExternalAPI.expected similarity index 100% rename from javascript/ql/test/query-tests/Security/CWE-020/UntrustedDataToExternalAPI.expected rename to javascript/ql/test/query-tests/Security/CWE-020/UntrustedDataToExternalAPI/UntrustedDataToExternalAPI.expected diff --git a/javascript/ql/test/query-tests/Security/CWE-020/UntrustedDataToExternalAPI.qlref b/javascript/ql/test/query-tests/Security/CWE-020/UntrustedDataToExternalAPI/UntrustedDataToExternalAPI.qlref similarity index 100% rename from javascript/ql/test/query-tests/Security/CWE-020/UntrustedDataToExternalAPI.qlref rename to javascript/ql/test/query-tests/Security/CWE-020/UntrustedDataToExternalAPI/UntrustedDataToExternalAPI.qlref diff --git a/javascript/ql/test/query-tests/Security/CWE-020/tst-UntrustedDataToExternalAPI.js b/javascript/ql/test/query-tests/Security/CWE-020/UntrustedDataToExternalAPI/tst-UntrustedDataToExternalAPI.js similarity index 100% rename from javascript/ql/test/query-tests/Security/CWE-020/tst-UntrustedDataToExternalAPI.js rename to javascript/ql/test/query-tests/Security/CWE-020/UntrustedDataToExternalAPI/tst-UntrustedDataToExternalAPI.js diff --git a/javascript/ql/test/query-tests/Security/CWE-020/UselessCharacterEscape.expected b/javascript/ql/test/query-tests/Security/CWE-020/UselessCharacterEscape/UselessCharacterEscape.expected similarity index 100% rename from javascript/ql/test/query-tests/Security/CWE-020/UselessCharacterEscape.expected rename to javascript/ql/test/query-tests/Security/CWE-020/UselessCharacterEscape/UselessCharacterEscape.expected diff --git a/javascript/ql/test/query-tests/Security/CWE-020/UselessCharacterEscape.ql b/javascript/ql/test/query-tests/Security/CWE-020/UselessCharacterEscape/UselessCharacterEscape.ql similarity index 100% rename from javascript/ql/test/query-tests/Security/CWE-020/UselessCharacterEscape.ql rename to javascript/ql/test/query-tests/Security/CWE-020/UselessCharacterEscape/UselessCharacterEscape.ql diff --git a/javascript/ql/test/query-tests/Security/CWE-020/UselessRegExpCharacterEscape.expected b/javascript/ql/test/query-tests/Security/CWE-020/UselessCharacterEscape/UselessRegExpCharacterEscape.expected similarity index 100% rename from javascript/ql/test/query-tests/Security/CWE-020/UselessRegExpCharacterEscape.expected rename to javascript/ql/test/query-tests/Security/CWE-020/UselessCharacterEscape/UselessRegExpCharacterEscape.expected diff --git a/javascript/ql/test/query-tests/Security/CWE-020/UselessRegExpCharacterEscape.qlref b/javascript/ql/test/query-tests/Security/CWE-020/UselessCharacterEscape/UselessRegExpCharacterEscape.qlref similarity index 100% rename from javascript/ql/test/query-tests/Security/CWE-020/UselessRegExpCharacterEscape.qlref rename to javascript/ql/test/query-tests/Security/CWE-020/UselessCharacterEscape/UselessRegExpCharacterEscape.qlref diff --git a/javascript/ql/test/query-tests/Security/CWE-020/tst-escapes.js b/javascript/ql/test/query-tests/Security/CWE-020/UselessCharacterEscape/tst-escapes.js similarity index 100% rename from javascript/ql/test/query-tests/Security/CWE-020/tst-escapes.js rename to javascript/ql/test/query-tests/Security/CWE-020/UselessCharacterEscape/tst-escapes.js From e2badab2513449e1ca75c3454f7563bb3ed0ea8d Mon Sep 17 00:00:00 2001 From: Erik Krogh Kristensen Date: Fri, 8 Apr 2022 11:00:27 +0200 Subject: [PATCH 03/11] update expected output after test reorganization --- .../MissingRegExpAnchor/MissingRegExpAnchor.expected | 10 ---------- .../UselessCharacterEscape.expected | 3 --- .../UselessRegExpCharacterEscape.expected | 3 --- 3 files changed, 16 deletions(-) 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 abc97d625d92..0554d8263838 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 @@ -1,13 +1,3 @@ -| tst-IncompleteHostnameRegExp.js:2:2:2:24 | /^http: ... le.com/ | This hostname pattern may match any domain name, as it is missing a '$' or '/' at the end. | -| tst-IncompleteHostnameRegExp.js:3:2:3:29 | /^http: ... le.com/ | This hostname pattern may match any domain name, as it is missing a '$' or '/' at the end. | -| tst-IncompleteHostnameRegExp.js:5:2:5:29 | /^http: ... le.net/ | This hostname pattern may match any domain name, as it is missing a '$' or '/' at the end. | -| tst-IncompleteHostnameRegExp.js:6:2:6:43 | /^http: ... b).com/ | This hostname pattern may match any domain name, as it is missing a '$' or '/' at the end. | -| tst-IncompleteHostnameRegExp.js:11:13:11:38 | "^http: ... le.com" | This hostname pattern may match any domain name, as it is missing a '$' or '/' at the end. | -| tst-IncompleteHostnameRegExp.js:12:14:12:39 | "^http: ... le.com" | This hostname pattern may match any domain name, as it is missing a '$' or '/' at the end. | -| tst-IncompleteHostnameRegExp.js:15:22:15:47 | "^http: ... le.com" | This hostname pattern may match any domain name, as it is missing a '$' or '/' at the end. | -| tst-IncompleteHostnameRegExp.js:19:17:19:35 | '^test.example.com' | This hostname pattern may match any domain name, as it is missing a '$' or '/' at the end. | -| tst-IncompleteHostnameRegExp.js:40:2:40:30 | /^https ... le.com/ | This hostname pattern may match any domain name, as it is missing a '$' or '/' at the end. | -| tst-IncompleteHostnameRegExp.js:55:13:55:39 | '^http: ... le.com' | This hostname pattern may match any domain name, as it is missing a '$' or '/' at the end. | | tst-SemiAnchoredRegExp.js:3:2:3:7 | /^a\|b/ | Misleading operator precedence. The subexpression '^a' is anchored at the beginning, but the other parts of this regular expression are not | | tst-SemiAnchoredRegExp.js:6:2:6:9 | /^a\|b\|c/ | Misleading operator precedence. The subexpression '^a' is anchored at the beginning, but the other parts of this regular expression are not | | tst-SemiAnchoredRegExp.js:12:2:12:9 | /^a\|(b)/ | Misleading operator precedence. The subexpression '^a' is anchored at the beginning, but the other parts of this regular expression are not | diff --git a/javascript/ql/test/query-tests/Security/CWE-020/UselessCharacterEscape/UselessCharacterEscape.expected b/javascript/ql/test/query-tests/Security/CWE-020/UselessCharacterEscape/UselessCharacterEscape.expected index 39084b5867e8..6cd6e27b0edc 100644 --- a/javascript/ql/test/query-tests/Security/CWE-020/UselessCharacterEscape/UselessCharacterEscape.expected +++ b/javascript/ql/test/query-tests/Security/CWE-020/UselessCharacterEscape/UselessCharacterEscape.expected @@ -1,6 +1,3 @@ -| tst-IncompleteHostnameRegExp.js:42:13:42:65 | '^http[ ... \\/(.+)' | The escape sequence '\\/' is equivalent to just '/'. | -| tst-SemiAnchoredRegExp.js:72:13:72:40 | '^good\\ ... \\\\.com' | The escape sequence '\\.' is equivalent to just '.'. | -| tst-SemiAnchoredRegExp.js:109:2:109:45 | /^((\\+\| ... ?\\d\\d)/ | The escape sequence '\\:' is equivalent to just ':'. | | tst-escapes.js:19:8:19:11 | "\\ " | The escape sequence '\\ ' is equivalent to just ' '. | | tst-escapes.js:20:1:20:54 | /\\a\\b\\c ... x\\y\\z"/ | The escape sequence '\\a' is equivalent to just 'a'. | | tst-escapes.js:20:1:20:54 | /\\a\\b\\c ... x\\y\\z"/ | The escape sequence '\\e' is equivalent to just 'e'. | diff --git a/javascript/ql/test/query-tests/Security/CWE-020/UselessCharacterEscape/UselessRegExpCharacterEscape.expected b/javascript/ql/test/query-tests/Security/CWE-020/UselessCharacterEscape/UselessRegExpCharacterEscape.expected index d37ee6a0e28c..f7badbcbd86a 100644 --- a/javascript/ql/test/query-tests/Security/CWE-020/UselessCharacterEscape/UselessRegExpCharacterEscape.expected +++ b/javascript/ql/test/query-tests/Security/CWE-020/UselessCharacterEscape/UselessRegExpCharacterEscape.expected @@ -1,6 +1,3 @@ -| tst-IncompleteHostnameRegExp.js:55:26:55:27 | '\\.' is equivalent to just '.', so the sequence may still represent a meta-character | The escape sequence '\\.' is equivalent to just '.', so the sequence may still represent a meta-character when it is used in a $@. | tst-IncompleteHostnameRegExp.js:55:13:55:39 | '^http: ... le.com' | regular expression | -| tst-SemiAnchoredRegExp.js:70:19:70:20 | '\\.' is equivalent to just '.', so the sequence may still represent a meta-character | The escape sequence '\\.' is equivalent to just '.', so the sequence may still represent a meta-character when it is used in a $@. | tst-SemiAnchoredRegExp.js:70:13:70:36 | '^good\\ ... r\\.com' | regular expression | -| tst-SemiAnchoredRegExp.js:70:31:70:32 | '\\.' is equivalent to just '.', so the sequence may still represent a meta-character | The escape sequence '\\.' is equivalent to just '.', so the sequence may still represent a meta-character when it is used in a $@. | tst-SemiAnchoredRegExp.js:70:13:70:36 | '^good\\ ... r\\.com' | regular expression | | tst-escapes.js:13:11:13:12 | '\\b' is a backspace, and not a word-boundary assertion | The escape sequence '\\b' is a backspace, and not a word-boundary assertion when it is used in a $@. | tst-escapes.js:13:8:13:61 | "\\a\\b\\c ... \\x\\y\\z" | regular expression | | tst-escapes.js:13:13:13:14 | '\\c' is equivalent to just 'c', so the sequence is not a character class | The escape sequence '\\c' is equivalent to just 'c', so the sequence is not a character class when it is used in a $@. | tst-escapes.js:13:8:13:61 | "\\a\\b\\c ... \\x\\y\\z" | regular expression | | tst-escapes.js:13:15:13:16 | '\\d' is equivalent to just 'd', so the sequence is not a character class | The escape sequence '\\d' is equivalent to just 'd', so the sequence is not a character class when it is used in a $@. | tst-escapes.js:13:8:13:61 | "\\a\\b\\c ... \\x\\y\\z" | regular expression | From 2d6d304d7c973c2a8334518a2f0edcd06f9a6c2d Mon Sep 17 00:00:00 2001 From: Erik Krogh Kristensen Date: Tue, 12 Apr 2022 13:15:31 +0200 Subject: [PATCH 04/11] add `InclusionTest` to `PostMessageEventSanitizer` --- .../javascript/dataflow/TaintTracking.qll | 18 ++++++++++++------ .../CWE-079/DomBasedXss/addEventListener.js | 13 +++++++++++++ 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/dataflow/TaintTracking.qll b/javascript/ql/lib/semmle/javascript/dataflow/TaintTracking.qll index 184e8a255a7f..45a68e39357d 100644 --- a/javascript/ql/lib/semmle/javascript/dataflow/TaintTracking.qll +++ b/javascript/ql/lib/semmle/javascript/dataflow/TaintTracking.qll @@ -1225,19 +1225,25 @@ module TaintTracking { * An equality test on `e.origin` or `e.source` where `e` is a `postMessage` event object, * considered as a sanitizer for `e`. */ - private class PostMessageEventSanitizer extends AdditionalSanitizerGuardNode, DataFlow::ValueNode { + private class PostMessageEventSanitizer extends AdditionalSanitizerGuardNode { VarAccess event; - override EqualityTest astNode; + boolean polarity; PostMessageEventSanitizer() { - exists(string prop | prop = "origin" or prop = "source" | - astNode.getAnOperand().(PropAccess).accesses(event, prop) and - event.mayReferToParameter(any(PostMessageEventHandler h).getEventParameter()) + event.mayReferToParameter(any(PostMessageEventHandler h).getEventParameter()) and + exists(DataFlow::PropRead read | read.accesses(event.flow(), ["origin", "source"]) | + exists(EqualityTest test | polarity = test.getPolarity() and this.getAstNode() = test | + test.getAnOperand().flow() = read + ) + or + exists(InclusionTest test | polarity = test.getPolarity() and this = test | + test.getContainedNode() = read + ) ) } override predicate sanitizes(boolean outcome, Expr e) { - outcome = astNode.getPolarity() and + outcome = polarity and e = event } diff --git a/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/addEventListener.js b/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/addEventListener.js index fbb1dcfe01d0..97d21371d082 100644 --- a/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/addEventListener.js +++ b/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/addEventListener.js @@ -14,4 +14,17 @@ function test() { } window.addEventListener("message", foo.bind(null, {data: 'items'})); + + window.onmessage = e => { + if (e.origin !== "https://foobar.com") { + return; + } + document.write(e.data); // OK - there is an origin check + } + + window.onmessage = e => { + if (mySet.includes(e.origin)) { + document.write(e.data); // OK - there is an origin check + } + } } From 591fcda8629ae65a5ea55df736819c540b972c6a Mon Sep 17 00:00:00 2001 From: Erik Krogh Kristensen Date: Tue, 12 Apr 2022 11:57:33 +0200 Subject: [PATCH 05/11] various improvements to the js/missing-origin-verification query --- .../CWE-020/PostMessageNoOriginCheck.qhelp | 25 +++-- .../CWE-020/PostMessageNoOriginCheck.ql | 99 ++++++++++--------- .../examples/postMessageInsufficientCheck.js | 14 --- .../examples/postMessageWithOriginCheck.js | 2 +- .../PostMessageNoOriginCheck.expected | 3 + .../PostMessageNoOriginCheck.qlref | 1 + .../Security/CWE-020/PostMessage/tst.js | 64 ++++++++++++ 7 files changed, 139 insertions(+), 69 deletions(-) delete mode 100644 javascript/ql/src/Security/CWE-020/examples/postMessageInsufficientCheck.js create mode 100644 javascript/ql/test/query-tests/Security/CWE-020/PostMessage/PostMessageNoOriginCheck.expected create mode 100644 javascript/ql/test/query-tests/Security/CWE-020/PostMessage/PostMessageNoOriginCheck.qlref create mode 100644 javascript/ql/test/query-tests/Security/CWE-020/PostMessage/tst.js diff --git a/javascript/ql/src/Security/CWE-020/PostMessageNoOriginCheck.qhelp b/javascript/ql/src/Security/CWE-020/PostMessageNoOriginCheck.qhelp index b8bcf242b55b..1151e59e8352 100644 --- a/javascript/ql/src/Security/CWE-020/PostMessageNoOriginCheck.qhelp +++ b/javascript/ql/src/Security/CWE-020/PostMessageNoOriginCheck.qhelp @@ -5,26 +5,31 @@ -

If you use cross-origin communication between Window objects and do expect to receive messages from other sites, always verify the sender's identity using the origin and possibly source properties of the recevied `MessageEvent`.

- -

Unexpected behaviours, like `DOM-based XSS` could occur, if the event handler for incoming data does not check the origin of the data received and handles the data in an unsafe way.

+

+The "message" event is used to send messages between windows. +An untrusted origin is allowed to send messages to a trusted window, and if the origin +is not checked that can lead to various security issues. +

-Always verify the sender's identity of incoming messages. +Always verify the origin of incoming messages.

- -

In the first example, the `MessageEvent.data` is passed to the `eval` function withouth checking the origin. This means that any window can send arbitrary messages that will be executed in the window receiving the message

+

+The example below uses a received message to execute some code. However, the +origin of the message is not checked, so it might be possible for an attacker +to execute arbitrary code. +

-

In the second example, the `MessageEvent.origin` is verified with an unsecure check. For example, using `event.origin.indexOf('www.example.com') > -1` can be bypassed because the string `www.example.com` could appear anywhere in `event.origin` (i.e. `www.example.com.mydomain.com`)

- - -

In the third example, the `MessageEvent.origin` is properly checked against a trusted origin.

+

+The example is fixed below, where the origin is checked to be trusted. +It is therefore not possible for an attacker to attack using an untrusted origin. +

 diff --git a/javascript/ql/src/Security/CWE-020/PostMessageNoOriginCheck.ql b/javascript/ql/src/Security/CWE-020/PostMessageNoOriginCheck.ql index 43d46d9133b3..44d6ae6a18ae 100644 --- a/javascript/ql/src/Security/CWE-020/PostMessageNoOriginCheck.ql +++ b/javascript/ql/src/Security/CWE-020/PostMessageNoOriginCheck.ql @@ -1,62 +1,73 @@ /** - * @name Missing `MessageEvent.origin` verification in `postMessage` handlers - * @description Missing the `MessageEvent.origin` verification in `postMessage` handlers, allows any windows to send arbitrary data to the `MessageEvent` listener. - * This could lead to unexpected behavior, especially when `MessageEvent.data` is used in an unsafe way. + * @name Missing origin verification in `postMessage` handler + * @description Missing origin verification in a `postMessage` handler allows any windows to send arbitrary data to the handler. * @kind problem * @problem.severity warning - * @precision high - * @id js/missing-postmessageorigin-verification + * @security-severity 5 + * @precision medium + * @id js/missing-origin-verification * @tags correctness * security * external/cwe/cwe-020 */ import javascript -import semmle.javascript.security.dataflow.DOM -/** - * A method call for the insecure functions used to verify the `MessageEvent.origin`. - */ -class InsufficientOriginChecks extends DataFlow::Node { - InsufficientOriginChecks() { - exists(DataFlow::Node node | - this.(StringOps::StartsWith).getSubstring() = node or - this.(StringOps::Includes).getSubstring() = node or - this.(StringOps::EndsWith).getSubstring() = node - ) +/** A function that handles "message" events. */ +class PostMessageHandler extends DataFlow::FunctionNode { + override PostMessageEventHandler astNode; + + /** Gets the parameter that contains the event. */ + DataFlow::ParameterNode getEventParameter() { + result = DataFlow::parameterNode(astNode.getEventParameter()) } } -/** - * A function handler for the `MessageEvent`. - */ -class PostMessageHandler extends DataFlow::FunctionNode { - PostMessageHandler() { this.getFunction() instanceof PostMessageEventHandler } +/** Gets a reference to the event from a postmessage `handler` */ +DataFlow::SourceNode event(DataFlow::TypeTracker t, PostMessageHandler handler) { + t.start() and + result = handler.getEventParameter() + or + exists(DataFlow::TypeTracker t2 | result = event(t2, handler).track(t2, t)) } -/** - * The `MessageEvent` parameter received by the handler - */ -class PostMessageEvent extends DataFlow::SourceNode { - PostMessageEvent() { exists(PostMessageHandler handler | this = handler.getParameter(0)) } - - /** - * Holds if an access on `MessageEvent.origin` is in an `EqualityTest` and there is no call of an insufficient verification method on `MessageEvent.origin` - */ - predicate hasOriginChecked() { - exists(EqualityTest test | - this.getAPropertyRead(["origin", "source"]).flowsToExpr(test.getAnOperand()) - ) - } +/** Gets a reference to the .origin from a postmessage event. */ +DataFlow::SourceNode origin(DataFlow::TypeTracker t, PostMessageHandler handler) { + t.start() and + result = event(DataFlow::TypeTracker::end(), handler).getAPropertyRead("origin") + or + result = + origin(t.continue(), handler) + .getAMethodCall([ + "toString", "toLowerCase", "toUpperCase", "toLocaleLowerCase", "toLocaleUpperCase" + ]) + or + exists(DataFlow::TypeTracker t2 | result = origin(t2, handler).track(t2, t)) +} - /** - * Holds if there is an insufficient method call (i.e indexOf) used to verify `MessageEvent.origin` - */ - predicate hasOriginInsufficientlyChecked() { - this.getAPropertyRead("origin").getAMethodCall*() instanceof InsufficientOriginChecks - } +/** Gets a reference to the .source from a postmessage event. */ +DataFlow::SourceNode source(DataFlow::TypeTracker t, PostMessageHandler handler) { + t.start() and + result = event(DataFlow::TypeTracker::end(), handler).getAPropertyRead("source") + or + exists(DataFlow::TypeTracker t2 | result = source(t2, handler).track(t2, t)) +} + +/** Gets a reference to the origin or the source of a postmessage event. */ +DataFlow::SourceNode sourceOrOrigin(PostMessageHandler handler) { + result = source(DataFlow::TypeTracker::end(), handler) or + result = origin(DataFlow::TypeTracker::end(), handler) +} + +/** Holds if there exists a check of the .origin or .source of the postmessage `handler`. */ +predicate hasOriginCheck(PostMessageHandler handler) { + // event.origin === "constant" + exists(EqualityTest test | sourceOrOrigin(handler).flowsToExpr(test.getAnOperand())) + or + // set.includes(event.source) + exists(InclusionTest test | sourceOrOrigin(handler).flowsTo(test.getContainedNode())) } -from PostMessageEvent event -where not event.hasOriginChecked() or event.hasOriginInsufficientlyChecked() -select event, "Missing or unsafe origin verification." +from PostMessageHandler handler +where not hasOriginCheck(handler) +select handler.getEventParameter(), "Postmessage handler has no origin check." diff --git a/javascript/ql/src/Security/CWE-020/examples/postMessageInsufficientCheck.js b/javascript/ql/src/Security/CWE-020/examples/postMessageInsufficientCheck.js deleted file mode 100644 index b92887f74e8e..000000000000 --- a/javascript/ql/src/Security/CWE-020/examples/postMessageInsufficientCheck.js +++ /dev/null @@ -1,14 +0,0 @@ -function postMessageHandler(event) { - let origin = event.origin.toLowerCase(); - - let host = window.location.host; - - // BAD - if (origin.indexOf(host) === -1) - return; - - - eval(event.data); -} - -window.addEventListener('message', postMessageHandler, false); \ No newline at end of file diff --git a/javascript/ql/src/Security/CWE-020/examples/postMessageWithOriginCheck.js b/javascript/ql/src/Security/CWE-020/examples/postMessageWithOriginCheck.js index e528f9802359..8dec29f2e184 100644 --- a/javascript/ql/src/Security/CWE-020/examples/postMessageWithOriginCheck.js +++ b/javascript/ql/src/Security/CWE-020/examples/postMessageWithOriginCheck.js @@ -1,7 +1,7 @@ function postMessageHandler(event) { console.log(event.origin) // GOOD: the origin property is checked - if (event.origin === 'www.example.com') { + if (event.origin === 'https://www.example.com') { // do something } } diff --git a/javascript/ql/test/query-tests/Security/CWE-020/PostMessage/PostMessageNoOriginCheck.expected b/javascript/ql/test/query-tests/Security/CWE-020/PostMessage/PostMessageNoOriginCheck.expected new file mode 100644 index 000000000000..58fb6ce79978 --- /dev/null +++ b/javascript/ql/test/query-tests/Security/CWE-020/PostMessage/PostMessageNoOriginCheck.expected @@ -0,0 +1,3 @@ +| tst.js:11:20:11:24 | event | Postmessage handler has no origin check. | +| tst.js:24:27:24:27 | e | Postmessage handler has no origin check. | +| tst.js:40:27:40:27 | e | Postmessage handler has no origin check. | diff --git a/javascript/ql/test/query-tests/Security/CWE-020/PostMessage/PostMessageNoOriginCheck.qlref b/javascript/ql/test/query-tests/Security/CWE-020/PostMessage/PostMessageNoOriginCheck.qlref new file mode 100644 index 000000000000..1b99d9b605a7 --- /dev/null +++ b/javascript/ql/test/query-tests/Security/CWE-020/PostMessage/PostMessageNoOriginCheck.qlref @@ -0,0 +1 @@ +Security/CWE-020/PostMessageNoOriginCheck.ql \ No newline at end of file diff --git a/javascript/ql/test/query-tests/Security/CWE-020/PostMessage/tst.js b/javascript/ql/test/query-tests/Security/CWE-020/PostMessage/tst.js new file mode 100644 index 000000000000..729f35637fc3 --- /dev/null +++ b/javascript/ql/test/query-tests/Security/CWE-020/PostMessage/tst.js @@ -0,0 +1,64 @@ +window.onmessage = event => { // OK - good origin check + let origin = event.origin.toLowerCase(); + + if (origin !== window.location.origin) { + return; + } + + eval(event.data); +} + +window.onmessage = event => { // NOT OK - no origin check + let origin = event.origin.toLowerCase(); + + console.log(origin); + eval(event.data); +} + +window.onmessage = event => { // OK - there is an origin check + if (event.origin === "https://www.example.com") { + // do something + } +} + +self.onmessage = function(e) { // NOT OK + Commands[e.data.cmd].apply(null, e.data.args); +}; + +window.onmessage = event => { // OK - there is an origin check + if (mySet.includes(event.origin)) { + // do something + } +} + +window.onmessage = event => { // OK - there is an origin check + if (mySet.includes(event.source)) { + // do something + } +} + +self.onmessage = function(e) { // NOT OK + Commands[e.data.cmd].apply(null, e.data.args); +}; + +window.addEventListener('message', function(e) { // OK - has a good origin check + if (is_sysend_post_message(e) && is_valid_origin(e.origin)) { + var payload = JSON.parse(e.data); + if (payload && payload.name === uniq_prefix) { + var data = unserialize(payload.data); + sysend.broadcast(payload.key, data); + } + } +}); + +function is_valid_origin(origin) { + if (!domains) { + warn("no domains configured"); + return true; + } + var valid = domains.includes(origin); + if (!valid) { + warn("invalid origin: " + origin); + } + return valid; +} \ No newline at end of file From bca4d1412942371735870f508f5e8955cd5cb7bf Mon Sep 17 00:00:00 2001 From: Erik Krogh Kristensen Date: Tue, 12 Apr 2022 14:37:43 +0200 Subject: [PATCH 06/11] rename files --- ...ostMessageNoOriginCheck.qhelp => MissingOriginCheck.qhelp} | 4 ++-- .../{PostMessageNoOriginCheck.ql => MissingOriginCheck.ql} | 2 +- .../{postMessageNoOriginCheck.js => MissingOriginCheckBad.js} | 0 ...ostMessageWithOriginCheck.js => MissingOriginCheckGood.js} | 0 .../MissingOriginCheck.expected} | 0 .../CWE-020/MissingOriginCheck/MissingOriginCheck.qlref | 1 + .../CWE-020/{PostMessage => MissingOriginCheck}/tst.js | 0 .../CWE-020/PostMessage/PostMessageNoOriginCheck.qlref | 1 - 8 files changed, 4 insertions(+), 4 deletions(-) rename javascript/ql/src/Security/CWE-020/{PostMessageNoOriginCheck.qhelp => MissingOriginCheck.qhelp} (92%) rename javascript/ql/src/Security/CWE-020/{PostMessageNoOriginCheck.ql => MissingOriginCheck.ql} (98%) rename javascript/ql/src/Security/CWE-020/examples/{postMessageNoOriginCheck.js => MissingOriginCheckBad.js} (100%) rename javascript/ql/src/Security/CWE-020/examples/{postMessageWithOriginCheck.js => MissingOriginCheckGood.js} (100%) rename javascript/ql/test/query-tests/Security/CWE-020/{PostMessage/PostMessageNoOriginCheck.expected => MissingOriginCheck/MissingOriginCheck.expected} (100%) create mode 100644 javascript/ql/test/query-tests/Security/CWE-020/MissingOriginCheck/MissingOriginCheck.qlref rename javascript/ql/test/query-tests/Security/CWE-020/{PostMessage => MissingOriginCheck}/tst.js (100%) delete mode 100644 javascript/ql/test/query-tests/Security/CWE-020/PostMessage/PostMessageNoOriginCheck.qlref diff --git a/javascript/ql/src/Security/CWE-020/PostMessageNoOriginCheck.qhelp b/javascript/ql/src/Security/CWE-020/MissingOriginCheck.qhelp similarity index 92% rename from javascript/ql/src/Security/CWE-020/PostMessageNoOriginCheck.qhelp rename to javascript/ql/src/Security/CWE-020/MissingOriginCheck.qhelp index 1151e59e8352..1bc5a87cbb12 100644 --- a/javascript/ql/src/Security/CWE-020/PostMessageNoOriginCheck.qhelp +++ b/javascript/ql/src/Security/CWE-020/MissingOriginCheck.qhelp @@ -24,13 +24,13 @@ The example below uses a received message to execute some code. However, the origin of the message is not checked, so it might be possible for an attacker to execute arbitrary code.

- +

The example is fixed below, where the origin is checked to be trusted. It is therefore not possible for an attacker to attack using an untrusted origin.

- + diff --git a/javascript/ql/src/Security/CWE-020/PostMessageNoOriginCheck.ql b/javascript/ql/src/Security/CWE-020/MissingOriginCheck.ql similarity index 98% rename from javascript/ql/src/Security/CWE-020/PostMessageNoOriginCheck.ql rename to javascript/ql/src/Security/CWE-020/MissingOriginCheck.ql index 44d6ae6a18ae..f4446ed75654 100644 --- a/javascript/ql/src/Security/CWE-020/PostMessageNoOriginCheck.ql +++ b/javascript/ql/src/Security/CWE-020/MissingOriginCheck.ql @@ -5,7 +5,7 @@ * @problem.severity warning * @security-severity 5 * @precision medium - * @id js/missing-origin-verification + * @id js/missing-origin-check * @tags correctness * security * external/cwe/cwe-020 diff --git a/javascript/ql/src/Security/CWE-020/examples/postMessageNoOriginCheck.js b/javascript/ql/src/Security/CWE-020/examples/MissingOriginCheckBad.js similarity index 100% rename from javascript/ql/src/Security/CWE-020/examples/postMessageNoOriginCheck.js rename to javascript/ql/src/Security/CWE-020/examples/MissingOriginCheckBad.js diff --git a/javascript/ql/src/Security/CWE-020/examples/postMessageWithOriginCheck.js b/javascript/ql/src/Security/CWE-020/examples/MissingOriginCheckGood.js similarity index 100% rename from javascript/ql/src/Security/CWE-020/examples/postMessageWithOriginCheck.js rename to javascript/ql/src/Security/CWE-020/examples/MissingOriginCheckGood.js diff --git a/javascript/ql/test/query-tests/Security/CWE-020/PostMessage/PostMessageNoOriginCheck.expected b/javascript/ql/test/query-tests/Security/CWE-020/MissingOriginCheck/MissingOriginCheck.expected similarity index 100% rename from javascript/ql/test/query-tests/Security/CWE-020/PostMessage/PostMessageNoOriginCheck.expected rename to javascript/ql/test/query-tests/Security/CWE-020/MissingOriginCheck/MissingOriginCheck.expected diff --git a/javascript/ql/test/query-tests/Security/CWE-020/MissingOriginCheck/MissingOriginCheck.qlref b/javascript/ql/test/query-tests/Security/CWE-020/MissingOriginCheck/MissingOriginCheck.qlref new file mode 100644 index 000000000000..02296c134e1a --- /dev/null +++ b/javascript/ql/test/query-tests/Security/CWE-020/MissingOriginCheck/MissingOriginCheck.qlref @@ -0,0 +1 @@ +Security/CWE-020/MissingOriginCheck.ql \ No newline at end of file diff --git a/javascript/ql/test/query-tests/Security/CWE-020/PostMessage/tst.js b/javascript/ql/test/query-tests/Security/CWE-020/MissingOriginCheck/tst.js similarity index 100% rename from javascript/ql/test/query-tests/Security/CWE-020/PostMessage/tst.js rename to javascript/ql/test/query-tests/Security/CWE-020/MissingOriginCheck/tst.js diff --git a/javascript/ql/test/query-tests/Security/CWE-020/PostMessage/PostMessageNoOriginCheck.qlref b/javascript/ql/test/query-tests/Security/CWE-020/PostMessage/PostMessageNoOriginCheck.qlref deleted file mode 100644 index 1b99d9b605a7..000000000000 --- a/javascript/ql/test/query-tests/Security/CWE-020/PostMessage/PostMessageNoOriginCheck.qlref +++ /dev/null @@ -1 +0,0 @@ -Security/CWE-020/PostMessageNoOriginCheck.ql \ No newline at end of file From df295e69d6e59933b450ea907e5455602633bc1a Mon Sep 17 00:00:00 2001 From: Erik Krogh Kristensen Date: Tue, 12 Apr 2022 14:37:51 +0200 Subject: [PATCH 07/11] add change-note --- .../2022-04-12-postmessage-origin-verification.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 javascript/ql/src/change-notes/2022-04-12-postmessage-origin-verification.md diff --git a/javascript/ql/src/change-notes/2022-04-12-postmessage-origin-verification.md b/javascript/ql/src/change-notes/2022-04-12-postmessage-origin-verification.md new file mode 100644 index 000000000000..f59652a8640a --- /dev/null +++ b/javascript/ql/src/change-notes/2022-04-12-postmessage-origin-verification.md @@ -0,0 +1,5 @@ +--- +category: newQuery +--- +* The `js/missing-origin-check` query has been added. It highlights "message" event handlers that do not check the origin of the event. + The query previously existed as the experimental `js/missing-postmessageorigin-verification` query. \ No newline at end of file From fe3d71ebc21dc225a37bcad3d1da3e2d0aed610a Mon Sep 17 00:00:00 2001 From: Erik Krogh Kristensen Date: Mon, 25 Apr 2022 14:07:01 +0200 Subject: [PATCH 08/11] fix qhelp: the window, not the origin, is sending the message Co-authored-by: Esben Sparre Andreasen --- javascript/ql/src/Security/CWE-020/MissingOriginCheck.qhelp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/javascript/ql/src/Security/CWE-020/MissingOriginCheck.qhelp b/javascript/ql/src/Security/CWE-020/MissingOriginCheck.qhelp index 1bc5a87cbb12..ab551927ed19 100644 --- a/javascript/ql/src/Security/CWE-020/MissingOriginCheck.qhelp +++ b/javascript/ql/src/Security/CWE-020/MissingOriginCheck.qhelp @@ -7,8 +7,7 @@

The "message" event is used to send messages between windows. -An untrusted origin is allowed to send messages to a trusted window, and if the origin -is not checked that can lead to various security issues. +An untrusted window can send a message to a trusted window, and it is up to the receiver to verify the legitimacy of the message. One way of doing that verification is to check the origin of the message ensure that it origins from a trusted window.

From 0a26e891a22ad57a29138150fc73f90bcfe5c37b Mon Sep 17 00:00:00 2001 From: Erik Krogh Kristensen Date: Mon, 25 Apr 2022 15:28:50 +0200 Subject: [PATCH 09/11] include startsWith/endsWith checks in js/missing-origin-check --- .../ql/src/Security/CWE-020/MissingOriginCheck.ql | 10 ++++++++++ .../Security/CWE-020/MissingOriginCheck/tst.js | 8 +++++++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/javascript/ql/src/Security/CWE-020/MissingOriginCheck.ql b/javascript/ql/src/Security/CWE-020/MissingOriginCheck.ql index f4446ed75654..543d3996abbe 100644 --- a/javascript/ql/src/Security/CWE-020/MissingOriginCheck.ql +++ b/javascript/ql/src/Security/CWE-020/MissingOriginCheck.ql @@ -66,6 +66,16 @@ predicate hasOriginCheck(PostMessageHandler handler) { or // set.includes(event.source) exists(InclusionTest test | sourceOrOrigin(handler).flowsTo(test.getContainedNode())) + or + // "safeOrigin".startsWith(event.origin) + exists(StringOps::StartsWith starts | + origin(DataFlow::TypeTracker::end(), handler).flowsTo(starts.getSubstring()) + ) + or + // "safeOrigin".endsWith(event.origin) + exists(StringOps::EndsWith ends | + origin(DataFlow::TypeTracker::end(), handler).flowsTo(ends.getSubstring()) + ) } from PostMessageHandler handler diff --git a/javascript/ql/test/query-tests/Security/CWE-020/MissingOriginCheck/tst.js b/javascript/ql/test/query-tests/Security/CWE-020/MissingOriginCheck/tst.js index 729f35637fc3..6e5c0ce6a14a 100644 --- a/javascript/ql/test/query-tests/Security/CWE-020/MissingOriginCheck/tst.js +++ b/javascript/ql/test/query-tests/Security/CWE-020/MissingOriginCheck/tst.js @@ -61,4 +61,10 @@ function is_valid_origin(origin) { warn("invalid origin: " + origin); } return valid; -} \ No newline at end of file +} + +window.onmessage = event => { // OK - the check is OK + if ("https://www.example.com".startsWith(event.origin)) { + // do something + } +} From 2d7c7ff372a2d25896ce7a241f68f5849dc45b6b Mon Sep 17 00:00:00 2001 From: Erik Krogh Kristensen Date: Thu, 5 May 2022 13:03:35 +0200 Subject: [PATCH 10/11] apply suggestions from doc review Co-authored-by: mc <42146119+mchammer01@users.noreply.github.com> --- .../ql/src/Security/CWE-020/MissingOriginCheck.qhelp | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/javascript/ql/src/Security/CWE-020/MissingOriginCheck.qhelp b/javascript/ql/src/Security/CWE-020/MissingOriginCheck.qhelp index ab551927ed19..e79bbbe7d75c 100644 --- a/javascript/ql/src/Security/CWE-020/MissingOriginCheck.qhelp +++ b/javascript/ql/src/Security/CWE-020/MissingOriginCheck.qhelp @@ -7,7 +7,7 @@

The "message" event is used to send messages between windows. -An untrusted window can send a message to a trusted window, and it is up to the receiver to verify the legitimacy of the message. One way of doing that verification is to check the origin of the message ensure that it origins from a trusted window. +An untrusted window can send a message to a trusted window, and it is up to the receiver to verify the legitimacy of the message. One way of performing that verification is to check the origin of the message ensure that it originates from a trusted window.

@@ -27,7 +27,7 @@ to execute arbitrary code.

The example is fixed below, where the origin is checked to be trusted. -It is therefore not possible for an attacker to attack using an untrusted origin. +It is therefore not possible for a malicious user to attack using an untrusted origin.

@@ -35,10 +35,9 @@ It is therefore not possible for an attacker to attack using an untrusted origin -
  • CWE-020: Improper Input Validation
    • -
  • Window.postMessage()
    • -
  • Web-message manipulation
    • -
  • The pitfalls of postMessage
    • +
  • Window.postMessage().
    • +
  • Web message manipulation.
    • +
  • The pitfalls of postMessage.
    •  From 58db9226dc153384bc4be8051a8c5c0effb5a3a1 Mon Sep 17 00:00:00 2001 From: Erik Krogh Kristensen Date: Thu, 5 May 2022 14:24:45 +0200 Subject: [PATCH 11/11] add missing word in qhelp --- javascript/ql/src/Security/CWE-020/MissingOriginCheck.qhelp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/javascript/ql/src/Security/CWE-020/MissingOriginCheck.qhelp b/javascript/ql/src/Security/CWE-020/MissingOriginCheck.qhelp index e79bbbe7d75c..cb048b0c7713 100644 --- a/javascript/ql/src/Security/CWE-020/MissingOriginCheck.qhelp +++ b/javascript/ql/src/Security/CWE-020/MissingOriginCheck.qhelp @@ -27,7 +27,7 @@ to execute arbitrary code.

    The example is fixed below, where the origin is checked to be trusted. -It is therefore not possible for a malicious user to attack using an untrusted origin. +It is therefore not possible for a malicious user to perform an attack using an untrusted origin.