From c4b717b86c2bd82e3f8b0948520a0ac096d8e6f0 Mon Sep 17 00:00:00 2001 From: Napalys Date: Fri, 14 Mar 2025 08:48:45 +0100 Subject: [PATCH 1/6] Added test case for `escape`. --- .../Security/CWE-022/TaintedPath/TaintedPath.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/TaintedPath.js b/javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/TaintedPath.js index ba57e930f03f..e0cd0b95482e 100644 --- a/javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/TaintedPath.js +++ b/javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/TaintedPath.js @@ -208,3 +208,10 @@ var server = http.createServer(function(req, res) { } }); +var srv = http.createServer(function(req, res) { + let path = url.parse(req.url, true).query.path; // $ MISSING: Source + const improperEscape = escape(path); + res.write(fs.readFileSync(improperEscape)); // $ MISSING: Alert + const improperEscape2 = unescape(path); + res.write(fs.readFileSync(improperEscape2)); // $ MISSING: Alert +}); From dc262236f4840ecffde1e26005087c829d4c9591 Mon Sep 17 00:00:00 2001 From: Napalys Date: Fri, 14 Mar 2025 11:43:22 +0100 Subject: [PATCH 2/6] Enhance taint tracking by including `escape` and `unescape` in TaintedPath customizations. --- .../dataflow/TaintedPathCustomizations.qll | 5 +++- .../CWE-022/TaintedPath/TaintedPath.expected | 27 +++++++++++++++++++ .../CWE-022/TaintedPath/TaintedPath.js | 6 ++--- 3 files changed, 34 insertions(+), 4 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/TaintedPathCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/TaintedPathCustomizations.qll index dc23b895a4f6..366d66e5b43c 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/TaintedPathCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/TaintedPathCustomizations.qll @@ -892,7 +892,10 @@ module TaintedPath { TaintTracking::uriStep(node1, node2) or exists(DataFlow::CallNode decode | - decode.getCalleeName() = "decodeURIComponent" or decode.getCalleeName() = "decodeURI" + decode.getCalleeName() = "decodeURIComponent" or + decode.getCalleeName() = "decodeURI" or + decode.getCalleeName() = "escape" or + decode.getCalleeName() = "unescape" | node1 = decode.getArgument(0) and node2 = decode diff --git a/javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/TaintedPath.expected b/javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/TaintedPath.expected index 2c19dc96c898..17c3e12dedda 100644 --- a/javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/TaintedPath.expected +++ b/javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/TaintedPath.expected @@ -45,6 +45,8 @@ | TaintedPath.js:195:29:195:85 | path.re ... '), '') | TaintedPath.js:191:24:191:30 | req.url | TaintedPath.js:195:29:195:85 | path.re ... '), '') | This path depends on a $@. | TaintedPath.js:191:24:191:30 | req.url | user-provided value | | TaintedPath.js:202:29:202:68 | path.re ... '), '') | TaintedPath.js:200:24:200:30 | req.url | TaintedPath.js:202:29:202:68 | path.re ... '), '') | This path depends on a $@. | TaintedPath.js:200:24:200:30 | req.url | user-provided value | | TaintedPath.js:205:31:205:69 | path.re ... '), '') | TaintedPath.js:200:24:200:30 | req.url | TaintedPath.js:205:31:205:69 | path.re ... '), '') | This path depends on a $@. | TaintedPath.js:200:24:200:30 | req.url | user-provided value | +| TaintedPath.js:214:29:214:42 | improperEscape | TaintedPath.js:212:24:212:30 | req.url | TaintedPath.js:214:29:214:42 | improperEscape | This path depends on a $@. | TaintedPath.js:212:24:212:30 | req.url | user-provided value | +| TaintedPath.js:216:29:216:43 | improperEscape2 | TaintedPath.js:212:24:212:30 | req.url | TaintedPath.js:216:29:216:43 | improperEscape2 | This path depends on a $@. | TaintedPath.js:212:24:212:30 | req.url | user-provided value | | examples/TaintedPath.js:10:29:10:43 | ROOT + filePath | examples/TaintedPath.js:8:28:8:34 | req.url | examples/TaintedPath.js:10:29:10:43 | ROOT + filePath | This path depends on a $@. | examples/TaintedPath.js:8:28:8:34 | req.url | user-provided value | | express.js:8:20:8:32 | req.query.bar | express.js:8:20:8:32 | req.query.bar | express.js:8:20:8:32 | req.query.bar | This path depends on a $@. | express.js:8:20:8:32 | req.query.bar | user-provided value | | handlebars.js:11:32:11:39 | filePath | handlebars.js:29:46:29:60 | req.params.path | handlebars.js:11:32:11:39 | filePath | This path depends on a $@. | handlebars.js:29:46:29:60 | req.params.path | user-provided value | @@ -320,6 +322,18 @@ edges | TaintedPath.js:200:24:200:30 | req.url | TaintedPath.js:200:14:200:37 | url.par ... , true) | provenance | Config | | TaintedPath.js:202:29:202:32 | path | TaintedPath.js:202:29:202:68 | path.re ... '), '') | provenance | Config | | TaintedPath.js:205:31:205:34 | path | TaintedPath.js:205:31:205:69 | path.re ... '), '') | provenance | Config | +| TaintedPath.js:212:7:212:48 | path | TaintedPath.js:213:33:213:36 | path | provenance | | +| TaintedPath.js:212:7:212:48 | path | TaintedPath.js:215:36:215:39 | path | provenance | | +| TaintedPath.js:212:14:212:37 | url.par ... , true) | TaintedPath.js:212:14:212:43 | url.par ... ).query | provenance | Config | +| TaintedPath.js:212:14:212:43 | url.par ... ).query | TaintedPath.js:212:14:212:48 | url.par ... ry.path | provenance | Config | +| TaintedPath.js:212:14:212:48 | url.par ... ry.path | TaintedPath.js:212:7:212:48 | path | provenance | | +| TaintedPath.js:212:24:212:30 | req.url | TaintedPath.js:212:14:212:37 | url.par ... , true) | provenance | Config | +| TaintedPath.js:213:9:213:37 | improperEscape | TaintedPath.js:214:29:214:42 | improperEscape | provenance | | +| TaintedPath.js:213:26:213:37 | escape(path) | TaintedPath.js:213:9:213:37 | improperEscape | provenance | | +| TaintedPath.js:213:33:213:36 | path | TaintedPath.js:213:26:213:37 | escape(path) | provenance | Config | +| TaintedPath.js:215:9:215:40 | improperEscape2 | TaintedPath.js:216:29:216:43 | improperEscape2 | provenance | | +| TaintedPath.js:215:27:215:40 | unescape(path) | TaintedPath.js:215:9:215:40 | improperEscape2 | provenance | | +| TaintedPath.js:215:36:215:39 | path | TaintedPath.js:215:27:215:40 | unescape(path) | provenance | Config | | examples/TaintedPath.js:8:7:8:52 | filePath | examples/TaintedPath.js:10:36:10:43 | filePath | provenance | | | examples/TaintedPath.js:8:18:8:41 | url.par ... , true) | examples/TaintedPath.js:8:18:8:47 | url.par ... ).query | provenance | Config | | examples/TaintedPath.js:8:18:8:47 | url.par ... ).query | examples/TaintedPath.js:8:18:8:52 | url.par ... ry.path | provenance | Config | @@ -780,6 +794,19 @@ nodes | TaintedPath.js:202:29:202:68 | path.re ... '), '') | semmle.label | path.re ... '), '') | | TaintedPath.js:205:31:205:34 | path | semmle.label | path | | TaintedPath.js:205:31:205:69 | path.re ... '), '') | semmle.label | path.re ... '), '') | +| TaintedPath.js:212:7:212:48 | path | semmle.label | path | +| TaintedPath.js:212:14:212:37 | url.par ... , true) | semmle.label | url.par ... , true) | +| TaintedPath.js:212:14:212:43 | url.par ... ).query | semmle.label | url.par ... ).query | +| TaintedPath.js:212:14:212:48 | url.par ... ry.path | semmle.label | url.par ... ry.path | +| TaintedPath.js:212:24:212:30 | req.url | semmle.label | req.url | +| TaintedPath.js:213:9:213:37 | improperEscape | semmle.label | improperEscape | +| TaintedPath.js:213:26:213:37 | escape(path) | semmle.label | escape(path) | +| TaintedPath.js:213:33:213:36 | path | semmle.label | path | +| TaintedPath.js:214:29:214:42 | improperEscape | semmle.label | improperEscape | +| TaintedPath.js:215:9:215:40 | improperEscape2 | semmle.label | improperEscape2 | +| TaintedPath.js:215:27:215:40 | unescape(path) | semmle.label | unescape(path) | +| TaintedPath.js:215:36:215:39 | path | semmle.label | path | +| TaintedPath.js:216:29:216:43 | improperEscape2 | semmle.label | improperEscape2 | | examples/TaintedPath.js:8:7:8:52 | filePath | semmle.label | filePath | | examples/TaintedPath.js:8:18:8:41 | url.par ... , true) | semmle.label | url.par ... , true) | | examples/TaintedPath.js:8:18:8:47 | url.par ... ).query | semmle.label | url.par ... ).query | diff --git a/javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/TaintedPath.js b/javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/TaintedPath.js index e0cd0b95482e..7e9d09b0d4af 100644 --- a/javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/TaintedPath.js +++ b/javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/TaintedPath.js @@ -209,9 +209,9 @@ var server = http.createServer(function(req, res) { }); var srv = http.createServer(function(req, res) { - let path = url.parse(req.url, true).query.path; // $ MISSING: Source + let path = url.parse(req.url, true).query.path; // $ Source const improperEscape = escape(path); - res.write(fs.readFileSync(improperEscape)); // $ MISSING: Alert + res.write(fs.readFileSync(improperEscape)); // $ Alert const improperEscape2 = unescape(path); - res.write(fs.readFileSync(improperEscape2)); // $ MISSING: Alert + res.write(fs.readFileSync(improperEscape2)); // $ Alert }); From 4c77ee2f4f41afc8cfeac797d826e1458531a648 Mon Sep 17 00:00:00 2001 From: Napalys Date: Fri, 14 Mar 2025 14:27:14 +0100 Subject: [PATCH 3/6] Added change note. --- javascript/ql/lib/change-notes/2025-03-14-escape.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 javascript/ql/lib/change-notes/2025-03-14-escape.md diff --git a/javascript/ql/lib/change-notes/2025-03-14-escape.md b/javascript/ql/lib/change-notes/2025-03-14-escape.md new file mode 100644 index 000000000000..334fd6cc04d4 --- /dev/null +++ b/javascript/ql/lib/change-notes/2025-03-14-escape.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* Added additional flow step for `unescape()` and `escape()`. From 37e02e4261167f4cc661759b12dcdf954afbfee0 Mon Sep 17 00:00:00 2001 From: Napalys Date: Fri, 14 Mar 2025 14:49:45 +0100 Subject: [PATCH 4/6] Added `escape` as `StringManipulationTaintStep`. --- .../lib/semmle/javascript/dataflow/TaintTracking.qll | 3 ++- .../Security/CWE-079/DomBasedXss/Xss.expected | 12 ++++++++++++ .../DomBasedXss/XssWithAdditionalSources.expected | 10 ++++++++++ .../CWE-079/DomBasedXss/string-manipulations.js | 4 ++-- 4 files changed, 26 insertions(+), 3 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/dataflow/TaintTracking.qll b/javascript/ql/lib/semmle/javascript/dataflow/TaintTracking.qll index 2dd1f622fb5b..862cf1b84274 100644 --- a/javascript/ql/lib/semmle/javascript/dataflow/TaintTracking.qll +++ b/javascript/ql/lib/semmle/javascript/dataflow/TaintTracking.qll @@ -494,7 +494,8 @@ module TaintTracking { succ = c and c = DataFlow::globalVarRef([ - "encodeURI", "decodeURI", "encodeURIComponent", "decodeURIComponent", "unescape" + "encodeURI", "decodeURI", "encodeURIComponent", "decodeURIComponent", "unescape", + "escape" ]).getACall() and pred = c.getArgument(0) ) diff --git a/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/Xss.expected b/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/Xss.expected index 7de1561f79e8..1943f6f625b2 100644 --- a/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/Xss.expected +++ b/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/Xss.expected @@ -120,6 +120,8 @@ | string-manipulations.js:8:16:8:48 | documen ... mLeft() | string-manipulations.js:8:16:8:37 | documen ... on.href | string-manipulations.js:8:16:8:48 | documen ... mLeft() | Cross-site scripting vulnerability due to $@. | string-manipulations.js:8:16:8:37 | documen ... on.href | user-provided value | | string-manipulations.js:9:16:9:58 | String. ... n.href) | string-manipulations.js:9:36:9:57 | documen ... on.href | string-manipulations.js:9:16:9:58 | String. ... n.href) | Cross-site scripting vulnerability due to $@. | string-manipulations.js:9:36:9:57 | documen ... on.href | user-provided value | | string-manipulations.js:10:16:10:45 | String( ... n.href) | string-manipulations.js:10:23:10:44 | documen ... on.href | string-manipulations.js:10:16:10:45 | String( ... n.href) | Cross-site scripting vulnerability due to $@. | string-manipulations.js:10:23:10:44 | documen ... on.href | user-provided value | +| string-manipulations.js:11:16:11:45 | escape( ... n.href) | string-manipulations.js:11:23:11:44 | documen ... on.href | string-manipulations.js:11:16:11:45 | escape( ... n.href) | Cross-site scripting vulnerability due to $@. | string-manipulations.js:11:23:11:44 | documen ... on.href | user-provided value | +| string-manipulations.js:12:16:12:61 | escape( ... href))) | string-manipulations.js:12:37:12:58 | documen ... on.href | string-manipulations.js:12:16:12:61 | escape( ... href))) | Cross-site scripting vulnerability due to $@. | string-manipulations.js:12:37:12:58 | documen ... on.href | user-provided value | | tainted-url-suffix-arguments.js:6:22:6:22 | y | tainted-url-suffix-arguments.js:11:17:11:36 | window.location.href | tainted-url-suffix-arguments.js:6:22:6:22 | y | Cross-site scripting vulnerability due to $@. | tainted-url-suffix-arguments.js:11:17:11:36 | window.location.href | user-provided value | | tooltip.jsx:10:25:10:30 | source | tooltip.jsx:6:20:6:30 | window.name | tooltip.jsx:10:25:10:30 | source | Cross-site scripting vulnerability due to $@. | tooltip.jsx:6:20:6:30 | window.name | user-provided value | | tooltip.jsx:11:25:11:30 | source | tooltip.jsx:6:20:6:30 | window.name | tooltip.jsx:11:25:11:30 | source | Cross-site scripting vulnerability due to $@. | tooltip.jsx:6:20:6:30 | window.name | user-provided value | @@ -490,6 +492,10 @@ edges | string-manipulations.js:8:16:8:37 | documen ... on.href | string-manipulations.js:8:16:8:48 | documen ... mLeft() | provenance | | | string-manipulations.js:9:36:9:57 | documen ... on.href | string-manipulations.js:9:16:9:58 | String. ... n.href) | provenance | | | string-manipulations.js:10:23:10:44 | documen ... on.href | string-manipulations.js:10:16:10:45 | String( ... n.href) | provenance | | +| string-manipulations.js:11:23:11:44 | documen ... on.href | string-manipulations.js:11:16:11:45 | escape( ... n.href) | provenance | | +| string-manipulations.js:12:23:12:60 | escape( ... .href)) | string-manipulations.js:12:16:12:61 | escape( ... href))) | provenance | | +| string-manipulations.js:12:30:12:59 | escape( ... n.href) | string-manipulations.js:12:23:12:60 | escape( ... .href)) | provenance | | +| string-manipulations.js:12:37:12:58 | documen ... on.href | string-manipulations.js:12:30:12:59 | escape( ... n.href) | provenance | | | tainted-url-suffix-arguments.js:3:17:3:17 | y | tainted-url-suffix-arguments.js:6:22:6:22 | y | provenance | | | tainted-url-suffix-arguments.js:11:11:11:36 | url | tainted-url-suffix-arguments.js:12:17:12:19 | url | provenance | | | tainted-url-suffix-arguments.js:11:17:11:36 | window.location.href | tainted-url-suffix-arguments.js:11:11:11:36 | url | provenance | | @@ -1116,6 +1122,12 @@ nodes | string-manipulations.js:9:36:9:57 | documen ... on.href | semmle.label | documen ... on.href | | string-manipulations.js:10:16:10:45 | String( ... n.href) | semmle.label | String( ... n.href) | | string-manipulations.js:10:23:10:44 | documen ... on.href | semmle.label | documen ... on.href | +| string-manipulations.js:11:16:11:45 | escape( ... n.href) | semmle.label | escape( ... n.href) | +| string-manipulations.js:11:23:11:44 | documen ... on.href | semmle.label | documen ... on.href | +| string-manipulations.js:12:16:12:61 | escape( ... href))) | semmle.label | escape( ... href))) | +| string-manipulations.js:12:23:12:60 | escape( ... .href)) | semmle.label | escape( ... .href)) | +| string-manipulations.js:12:30:12:59 | escape( ... n.href) | semmle.label | escape( ... n.href) | +| string-manipulations.js:12:37:12:58 | documen ... on.href | semmle.label | documen ... on.href | | tainted-url-suffix-arguments.js:3:17:3:17 | y | semmle.label | y | | tainted-url-suffix-arguments.js:6:22:6:22 | y | semmle.label | y | | tainted-url-suffix-arguments.js:11:11:11:36 | url | semmle.label | url | diff --git a/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/XssWithAdditionalSources.expected b/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/XssWithAdditionalSources.expected index eb961fc83dbf..da40eb89b0da 100644 --- a/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/XssWithAdditionalSources.expected +++ b/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/XssWithAdditionalSources.expected @@ -322,6 +322,12 @@ nodes | string-manipulations.js:9:36:9:57 | documen ... on.href | semmle.label | documen ... on.href | | string-manipulations.js:10:16:10:45 | String( ... n.href) | semmle.label | String( ... n.href) | | string-manipulations.js:10:23:10:44 | documen ... on.href | semmle.label | documen ... on.href | +| string-manipulations.js:11:16:11:45 | escape( ... n.href) | semmle.label | escape( ... n.href) | +| string-manipulations.js:11:23:11:44 | documen ... on.href | semmle.label | documen ... on.href | +| string-manipulations.js:12:16:12:61 | escape( ... href))) | semmle.label | escape( ... href))) | +| string-manipulations.js:12:23:12:60 | escape( ... .href)) | semmle.label | escape( ... .href)) | +| string-manipulations.js:12:30:12:59 | escape( ... n.href) | semmle.label | escape( ... n.href) | +| string-manipulations.js:12:37:12:58 | documen ... on.href | semmle.label | documen ... on.href | | tainted-url-suffix-arguments.js:3:17:3:17 | y | semmle.label | y | | tainted-url-suffix-arguments.js:6:22:6:22 | y | semmle.label | y | | tainted-url-suffix-arguments.js:11:11:11:36 | url | semmle.label | url | @@ -934,6 +940,10 @@ edges | string-manipulations.js:8:16:8:37 | documen ... on.href | string-manipulations.js:8:16:8:48 | documen ... mLeft() | provenance | | | string-manipulations.js:9:36:9:57 | documen ... on.href | string-manipulations.js:9:16:9:58 | String. ... n.href) | provenance | | | string-manipulations.js:10:23:10:44 | documen ... on.href | string-manipulations.js:10:16:10:45 | String( ... n.href) | provenance | | +| string-manipulations.js:11:23:11:44 | documen ... on.href | string-manipulations.js:11:16:11:45 | escape( ... n.href) | provenance | | +| string-manipulations.js:12:23:12:60 | escape( ... .href)) | string-manipulations.js:12:16:12:61 | escape( ... href))) | provenance | | +| string-manipulations.js:12:30:12:59 | escape( ... n.href) | string-manipulations.js:12:23:12:60 | escape( ... .href)) | provenance | | +| string-manipulations.js:12:37:12:58 | documen ... on.href | string-manipulations.js:12:30:12:59 | escape( ... n.href) | provenance | | | tainted-url-suffix-arguments.js:3:17:3:17 | y | tainted-url-suffix-arguments.js:6:22:6:22 | y | provenance | | | tainted-url-suffix-arguments.js:11:11:11:36 | url | tainted-url-suffix-arguments.js:12:17:12:19 | url | provenance | | | tainted-url-suffix-arguments.js:11:17:11:36 | window.location.href | tainted-url-suffix-arguments.js:11:11:11:36 | url | provenance | | diff --git a/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/string-manipulations.js b/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/string-manipulations.js index d20f88dba398..917cad0f9126 100644 --- a/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/string-manipulations.js +++ b/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/string-manipulations.js @@ -8,5 +8,5 @@ document.write(document.location.href.toUpperCase()); // $ Alert document.write(document.location.href.trimLeft()); // $ Alert document.write(String.fromCharCode(document.location.href)); // $ Alert document.write(String(document.location.href)); // $ Alert -document.write(escape(document.location.href)); // OK - for now -document.write(escape(escape(escape(document.location.href)))); // OK - for now +document.write(escape(document.location.href)); // $ SPURIOUS: Alert +document.write(escape(escape(escape(document.location.href)))); // $ SPURIOUS: Alert From 4a691b778bb5c16bc2d4dfcb3ba4f840608628cc Mon Sep 17 00:00:00 2001 From: Napalys Date: Fri, 14 Mar 2025 14:53:21 +0100 Subject: [PATCH 5/6] Added `escape` as `UriEncodingSanitizer` --- .../lib/semmle/javascript/security/dataflow/Xss.qll | 2 +- .../Security/CWE-079/DomBasedXss/Xss.expected | 12 ------------ .../DomBasedXss/XssWithAdditionalSources.expected | 10 ---------- .../CWE-079/DomBasedXss/string-manipulations.js | 4 ++-- 4 files changed, 3 insertions(+), 25 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/Xss.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/Xss.qll index 0d17c9d84943..41487b8c3b64 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/Xss.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/Xss.qll @@ -53,7 +53,7 @@ module Shared { class UriEncodingSanitizer extends Sanitizer, DataFlow::CallNode { UriEncodingSanitizer() { exists(string name | this = DataFlow::globalVarRef(name).getACall() | - name = "encodeURI" or name = "encodeURIComponent" + name in ["encodeURI", "encodeURIComponent", "escape"] ) } } diff --git a/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/Xss.expected b/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/Xss.expected index 1943f6f625b2..7de1561f79e8 100644 --- a/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/Xss.expected +++ b/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/Xss.expected @@ -120,8 +120,6 @@ | string-manipulations.js:8:16:8:48 | documen ... mLeft() | string-manipulations.js:8:16:8:37 | documen ... on.href | string-manipulations.js:8:16:8:48 | documen ... mLeft() | Cross-site scripting vulnerability due to $@. | string-manipulations.js:8:16:8:37 | documen ... on.href | user-provided value | | string-manipulations.js:9:16:9:58 | String. ... n.href) | string-manipulations.js:9:36:9:57 | documen ... on.href | string-manipulations.js:9:16:9:58 | String. ... n.href) | Cross-site scripting vulnerability due to $@. | string-manipulations.js:9:36:9:57 | documen ... on.href | user-provided value | | string-manipulations.js:10:16:10:45 | String( ... n.href) | string-manipulations.js:10:23:10:44 | documen ... on.href | string-manipulations.js:10:16:10:45 | String( ... n.href) | Cross-site scripting vulnerability due to $@. | string-manipulations.js:10:23:10:44 | documen ... on.href | user-provided value | -| string-manipulations.js:11:16:11:45 | escape( ... n.href) | string-manipulations.js:11:23:11:44 | documen ... on.href | string-manipulations.js:11:16:11:45 | escape( ... n.href) | Cross-site scripting vulnerability due to $@. | string-manipulations.js:11:23:11:44 | documen ... on.href | user-provided value | -| string-manipulations.js:12:16:12:61 | escape( ... href))) | string-manipulations.js:12:37:12:58 | documen ... on.href | string-manipulations.js:12:16:12:61 | escape( ... href))) | Cross-site scripting vulnerability due to $@. | string-manipulations.js:12:37:12:58 | documen ... on.href | user-provided value | | tainted-url-suffix-arguments.js:6:22:6:22 | y | tainted-url-suffix-arguments.js:11:17:11:36 | window.location.href | tainted-url-suffix-arguments.js:6:22:6:22 | y | Cross-site scripting vulnerability due to $@. | tainted-url-suffix-arguments.js:11:17:11:36 | window.location.href | user-provided value | | tooltip.jsx:10:25:10:30 | source | tooltip.jsx:6:20:6:30 | window.name | tooltip.jsx:10:25:10:30 | source | Cross-site scripting vulnerability due to $@. | tooltip.jsx:6:20:6:30 | window.name | user-provided value | | tooltip.jsx:11:25:11:30 | source | tooltip.jsx:6:20:6:30 | window.name | tooltip.jsx:11:25:11:30 | source | Cross-site scripting vulnerability due to $@. | tooltip.jsx:6:20:6:30 | window.name | user-provided value | @@ -492,10 +490,6 @@ edges | string-manipulations.js:8:16:8:37 | documen ... on.href | string-manipulations.js:8:16:8:48 | documen ... mLeft() | provenance | | | string-manipulations.js:9:36:9:57 | documen ... on.href | string-manipulations.js:9:16:9:58 | String. ... n.href) | provenance | | | string-manipulations.js:10:23:10:44 | documen ... on.href | string-manipulations.js:10:16:10:45 | String( ... n.href) | provenance | | -| string-manipulations.js:11:23:11:44 | documen ... on.href | string-manipulations.js:11:16:11:45 | escape( ... n.href) | provenance | | -| string-manipulations.js:12:23:12:60 | escape( ... .href)) | string-manipulations.js:12:16:12:61 | escape( ... href))) | provenance | | -| string-manipulations.js:12:30:12:59 | escape( ... n.href) | string-manipulations.js:12:23:12:60 | escape( ... .href)) | provenance | | -| string-manipulations.js:12:37:12:58 | documen ... on.href | string-manipulations.js:12:30:12:59 | escape( ... n.href) | provenance | | | tainted-url-suffix-arguments.js:3:17:3:17 | y | tainted-url-suffix-arguments.js:6:22:6:22 | y | provenance | | | tainted-url-suffix-arguments.js:11:11:11:36 | url | tainted-url-suffix-arguments.js:12:17:12:19 | url | provenance | | | tainted-url-suffix-arguments.js:11:17:11:36 | window.location.href | tainted-url-suffix-arguments.js:11:11:11:36 | url | provenance | | @@ -1122,12 +1116,6 @@ nodes | string-manipulations.js:9:36:9:57 | documen ... on.href | semmle.label | documen ... on.href | | string-manipulations.js:10:16:10:45 | String( ... n.href) | semmle.label | String( ... n.href) | | string-manipulations.js:10:23:10:44 | documen ... on.href | semmle.label | documen ... on.href | -| string-manipulations.js:11:16:11:45 | escape( ... n.href) | semmle.label | escape( ... n.href) | -| string-manipulations.js:11:23:11:44 | documen ... on.href | semmle.label | documen ... on.href | -| string-manipulations.js:12:16:12:61 | escape( ... href))) | semmle.label | escape( ... href))) | -| string-manipulations.js:12:23:12:60 | escape( ... .href)) | semmle.label | escape( ... .href)) | -| string-manipulations.js:12:30:12:59 | escape( ... n.href) | semmle.label | escape( ... n.href) | -| string-manipulations.js:12:37:12:58 | documen ... on.href | semmle.label | documen ... on.href | | tainted-url-suffix-arguments.js:3:17:3:17 | y | semmle.label | y | | tainted-url-suffix-arguments.js:6:22:6:22 | y | semmle.label | y | | tainted-url-suffix-arguments.js:11:11:11:36 | url | semmle.label | url | diff --git a/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/XssWithAdditionalSources.expected b/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/XssWithAdditionalSources.expected index da40eb89b0da..eb961fc83dbf 100644 --- a/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/XssWithAdditionalSources.expected +++ b/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/XssWithAdditionalSources.expected @@ -322,12 +322,6 @@ nodes | string-manipulations.js:9:36:9:57 | documen ... on.href | semmle.label | documen ... on.href | | string-manipulations.js:10:16:10:45 | String( ... n.href) | semmle.label | String( ... n.href) | | string-manipulations.js:10:23:10:44 | documen ... on.href | semmle.label | documen ... on.href | -| string-manipulations.js:11:16:11:45 | escape( ... n.href) | semmle.label | escape( ... n.href) | -| string-manipulations.js:11:23:11:44 | documen ... on.href | semmle.label | documen ... on.href | -| string-manipulations.js:12:16:12:61 | escape( ... href))) | semmle.label | escape( ... href))) | -| string-manipulations.js:12:23:12:60 | escape( ... .href)) | semmle.label | escape( ... .href)) | -| string-manipulations.js:12:30:12:59 | escape( ... n.href) | semmle.label | escape( ... n.href) | -| string-manipulations.js:12:37:12:58 | documen ... on.href | semmle.label | documen ... on.href | | tainted-url-suffix-arguments.js:3:17:3:17 | y | semmle.label | y | | tainted-url-suffix-arguments.js:6:22:6:22 | y | semmle.label | y | | tainted-url-suffix-arguments.js:11:11:11:36 | url | semmle.label | url | @@ -940,10 +934,6 @@ edges | string-manipulations.js:8:16:8:37 | documen ... on.href | string-manipulations.js:8:16:8:48 | documen ... mLeft() | provenance | | | string-manipulations.js:9:36:9:57 | documen ... on.href | string-manipulations.js:9:16:9:58 | String. ... n.href) | provenance | | | string-manipulations.js:10:23:10:44 | documen ... on.href | string-manipulations.js:10:16:10:45 | String( ... n.href) | provenance | | -| string-manipulations.js:11:23:11:44 | documen ... on.href | string-manipulations.js:11:16:11:45 | escape( ... n.href) | provenance | | -| string-manipulations.js:12:23:12:60 | escape( ... .href)) | string-manipulations.js:12:16:12:61 | escape( ... href))) | provenance | | -| string-manipulations.js:12:30:12:59 | escape( ... n.href) | string-manipulations.js:12:23:12:60 | escape( ... .href)) | provenance | | -| string-manipulations.js:12:37:12:58 | documen ... on.href | string-manipulations.js:12:30:12:59 | escape( ... n.href) | provenance | | | tainted-url-suffix-arguments.js:3:17:3:17 | y | tainted-url-suffix-arguments.js:6:22:6:22 | y | provenance | | | tainted-url-suffix-arguments.js:11:11:11:36 | url | tainted-url-suffix-arguments.js:12:17:12:19 | url | provenance | | | tainted-url-suffix-arguments.js:11:17:11:36 | window.location.href | tainted-url-suffix-arguments.js:11:11:11:36 | url | provenance | | diff --git a/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/string-manipulations.js b/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/string-manipulations.js index 917cad0f9126..895970f2b735 100644 --- a/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/string-manipulations.js +++ b/javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/string-manipulations.js @@ -8,5 +8,5 @@ document.write(document.location.href.toUpperCase()); // $ Alert document.write(document.location.href.trimLeft()); // $ Alert document.write(String.fromCharCode(document.location.href)); // $ Alert document.write(String(document.location.href)); // $ Alert -document.write(escape(document.location.href)); // $ SPURIOUS: Alert -document.write(escape(escape(escape(document.location.href)))); // $ SPURIOUS: Alert +document.write(escape(document.location.href)); +document.write(escape(escape(escape(document.location.href)))); From 478e32cbe52e793c3b081747c633ea88dc05cd88 Mon Sep 17 00:00:00 2001 From: Napalys Klicius Date: Mon, 17 Mar 2025 10:13:00 +0100 Subject: [PATCH 6/6] Update javascript/ql/lib/semmle/javascript/security/dataflow/TaintedPathCustomizations.qll Co-authored-by: Asger F --- .../security/dataflow/TaintedPathCustomizations.qll | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/TaintedPathCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/TaintedPathCustomizations.qll index 366d66e5b43c..f863b86a3b57 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/TaintedPathCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/TaintedPathCustomizations.qll @@ -892,10 +892,13 @@ module TaintedPath { TaintTracking::uriStep(node1, node2) or exists(DataFlow::CallNode decode | - decode.getCalleeName() = "decodeURIComponent" or - decode.getCalleeName() = "decodeURI" or - decode.getCalleeName() = "escape" or - decode.getCalleeName() = "unescape" + decode = + DataFlow::globalVarRef([ + "decodeURIComponent", + "decodeURI", + "escape", + "unescape" + ]).getACall() | node1 = decode.getArgument(0) and node2 = decode