Skip to content

Commit 749a056

Browse files
NapalysNapalys
authored
Merge pull request #19027 from Napalys/js/escape
JS: Add support for `escape`
2 parents 9134f79 + 478e32c commit 749a056

File tree

7 files changed

+50
-5
lines changed

7 files changed

+50
-5
lines changed

‎javascript/ql/lib/change-notes/2025-03-14-escape.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Added additional flow step for `unescape()` and `escape()`.

‎javascript/ql/lib/semmle/javascript/dataflow/TaintTracking.qll

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -494,7 +494,8 @@ module TaintTracking {
494494
succ = c and
495495
c =
496496
DataFlow::globalVarRef([
497-
"encodeURI", "decodeURI", "encodeURIComponent", "decodeURIComponent", "unescape"
497+
"encodeURI", "decodeURI", "encodeURIComponent", "decodeURIComponent", "unescape",
498+
"escape"
498499
]).getACall() and
499500
pred = c.getArgument(0)
500501
)

‎javascript/ql/lib/semmle/javascript/security/dataflow/TaintedPathCustomizations.qll

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -892,7 +892,13 @@ module TaintedPath {
892892
TaintTracking::uriStep(node1, node2)
893893
or
894894
exists(DataFlow::CallNode decode |
895-
decode.getCalleeName() = "decodeURIComponent" or decode.getCalleeName() = "decodeURI"
895+
decode =
896+
DataFlow::globalVarRef([
897+
"decodeURIComponent",
898+
"decodeURI",
899+
"escape",
900+
"unescape"
901+
]).getACall()
896902
|
897903
node1 = decode.getArgument(0) and
898904
node2 = decode

‎javascript/ql/lib/semmle/javascript/security/dataflow/Xss.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ module Shared {
5353
class UriEncodingSanitizer extends Sanitizer, DataFlow::CallNode {
5454
UriEncodingSanitizer() {
5555
exists(string name | this = DataFlow::globalVarRef(name).getACall() |
56-
name = "encodeURI" or name = "encodeURIComponent"
56+
name in ["encodeURI", "encodeURIComponent", "escape"]
5757
)
5858
}
5959
}

‎javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/TaintedPath.expected

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@
4545
| 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 |
4646
| 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 |
4747
| 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 |
48+
| 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 |
49+
| 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 |
4850
| 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 |
4951
| 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 |
5052
| 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
320322
| TaintedPath.js:200:24:200:30 | req.url | TaintedPath.js:200:14:200:37 | url.par ... , true) | provenance | Config |
321323
| TaintedPath.js:202:29:202:32 | path | TaintedPath.js:202:29:202:68 | path.re ... '), '') | provenance | Config |
322324
| TaintedPath.js:205:31:205:34 | path | TaintedPath.js:205:31:205:69 | path.re ... '), '') | provenance | Config |
325+
| TaintedPath.js:212:7:212:48 | path | TaintedPath.js:213:33:213:36 | path | provenance | |
326+
| TaintedPath.js:212:7:212:48 | path | TaintedPath.js:215:36:215:39 | path | provenance | |
327+
| TaintedPath.js:212:14:212:37 | url.par ... , true) | TaintedPath.js:212:14:212:43 | url.par ... ).query | provenance | Config |
328+
| TaintedPath.js:212:14:212:43 | url.par ... ).query | TaintedPath.js:212:14:212:48 | url.par ... ry.path | provenance | Config |
329+
| TaintedPath.js:212:14:212:48 | url.par ... ry.path | TaintedPath.js:212:7:212:48 | path | provenance | |
330+
| TaintedPath.js:212:24:212:30 | req.url | TaintedPath.js:212:14:212:37 | url.par ... , true) | provenance | Config |
331+
| TaintedPath.js:213:9:213:37 | improperEscape | TaintedPath.js:214:29:214:42 | improperEscape | provenance | |
332+
| TaintedPath.js:213:26:213:37 | escape(path) | TaintedPath.js:213:9:213:37 | improperEscape | provenance | |
333+
| TaintedPath.js:213:33:213:36 | path | TaintedPath.js:213:26:213:37 | escape(path) | provenance | Config |
334+
| TaintedPath.js:215:9:215:40 | improperEscape2 | TaintedPath.js:216:29:216:43 | improperEscape2 | provenance | |
335+
| TaintedPath.js:215:27:215:40 | unescape(path) | TaintedPath.js:215:9:215:40 | improperEscape2 | provenance | |
336+
| TaintedPath.js:215:36:215:39 | path | TaintedPath.js:215:27:215:40 | unescape(path) | provenance | Config |
323337
| examples/TaintedPath.js:8:7:8:52 | filePath | examples/TaintedPath.js:10:36:10:43 | filePath | provenance | |
324338
| examples/TaintedPath.js:8:18:8:41 | url.par ... , true) | examples/TaintedPath.js:8:18:8:47 | url.par ... ).query | provenance | Config |
325339
| 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
780794
| TaintedPath.js:202:29:202:68 | path.re ... '), '') | semmle.label | path.re ... '), '') |
781795
| TaintedPath.js:205:31:205:34 | path | semmle.label | path |
782796
| TaintedPath.js:205:31:205:69 | path.re ... '), '') | semmle.label | path.re ... '), '') |
797+
| TaintedPath.js:212:7:212:48 | path | semmle.label | path |
798+
| TaintedPath.js:212:14:212:37 | url.par ... , true) | semmle.label | url.par ... , true) |
799+
| TaintedPath.js:212:14:212:43 | url.par ... ).query | semmle.label | url.par ... ).query |
800+
| TaintedPath.js:212:14:212:48 | url.par ... ry.path | semmle.label | url.par ... ry.path |
801+
| TaintedPath.js:212:24:212:30 | req.url | semmle.label | req.url |
802+
| TaintedPath.js:213:9:213:37 | improperEscape | semmle.label | improperEscape |
803+
| TaintedPath.js:213:26:213:37 | escape(path) | semmle.label | escape(path) |
804+
| TaintedPath.js:213:33:213:36 | path | semmle.label | path |
805+
| TaintedPath.js:214:29:214:42 | improperEscape | semmle.label | improperEscape |
806+
| TaintedPath.js:215:9:215:40 | improperEscape2 | semmle.label | improperEscape2 |
807+
| TaintedPath.js:215:27:215:40 | unescape(path) | semmle.label | unescape(path) |
808+
| TaintedPath.js:215:36:215:39 | path | semmle.label | path |
809+
| TaintedPath.js:216:29:216:43 | improperEscape2 | semmle.label | improperEscape2 |
783810
| examples/TaintedPath.js:8:7:8:52 | filePath | semmle.label | filePath |
784811
| examples/TaintedPath.js:8:18:8:41 | url.par ... , true) | semmle.label | url.par ... , true) |
785812
| examples/TaintedPath.js:8:18:8:47 | url.par ... ).query | semmle.label | url.par ... ).query |

‎javascript/ql/test/query-tests/Security/CWE-022/TaintedPath/TaintedPath.js

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,3 +208,10 @@ var server = http.createServer(function(req, res) {
208208
}
209209
});
210210

211+
var srv = http.createServer(function(req, res) {
212+
let path = url.parse(req.url, true).query.path; // $ Source
213+
const improperEscape = escape(path);
214+
res.write(fs.readFileSync(improperEscape)); // $ Alert
215+
const improperEscape2 = unescape(path);
216+
res.write(fs.readFileSync(improperEscape2)); // $ Alert
217+
});

‎javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/string-manipulations.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,5 +8,5 @@ document.write(document.location.href.toUpperCase()); // $ Alert
88
document.write(document.location.href.trimLeft()); // $ Alert
99
document.write(String.fromCharCode(document.location.href)); // $ Alert
1010
document.write(String(document.location.href)); // $ Alert
11-
document.write(escape(document.location.href)); // OK - for now
12-
document.write(escape(escape(escape(document.location.href)))); // OK - for now
11+
document.write(escape(document.location.href));
12+
document.write(escape(escape(escape(document.location.href))));

0 commit comments

Comments
 (0)