hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2025-12-17 01:03:14 +01:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #19750 from Napalys/js/remove_encodeURI

Browse Source 
JS: remove `encodeURI` from sanitizer list of request forgery
This commit is contained in:
Napalys Klicius
2025-06-19 14:12:52 +02:00
committed by GitHub
parent 3f3a920a54 0d5f5104d1
commit f80651e78a
5 changed files with 42 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
javascript/ql/lib/change-notes/2025-06-13-remove-encodeuri.md Normal file
View File

@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* Removed `encodeURI` and `escape` functions from the sanitizer list for request forgery.

8
javascript/ql/lib/semmle/javascript/security/dataflow/RequestForgeryCustomizations.qll
View File

@@ -106,10 +106,12 @@ module RequestForgery {
private import Xss as Xss
/**
* A call to `encodeURI` or `encodeURIComponent`, viewed as a sanitizer for request forgery.
* A call to `encodeURIComponent`, viewed as a sanitizer for request forgery.
* These calls will escape "/" to "%2F", which is not a problem for request forgery.
* The result from calling `encodeURI` or `encodeURIComponent` is not a valid URL, and only makes sense
* The result from calling `encodeURIComponent` is not a valid URL, and only makes sense
* as a part of a URL.
*/
class UriEncodingSanitizer extends Sanitizer instanceof Xss::Shared::UriEncodingSanitizer { }
class UriEncodingSanitizer extends Sanitizer instanceof Xss::Shared::UriEncodingSanitizer {
UriEncodingSanitizer() { this.encodesPathSeparators() }
}
}

15
javascript/ql/lib/semmle/javascript/security/dataflow/Xss.qll
View File

@@ -47,15 +47,22 @@ module Shared {
}
/**
* A call to `encodeURI` or `encodeURIComponent`, viewed as a sanitizer for
* A call to `encodeURI`, `encodeURIComponent` or `escape`, viewed as a sanitizer for
* XSS vulnerabilities.
*/
class UriEncodingSanitizer extends Sanitizer, DataFlow::CallNode {
string name;
UriEncodingSanitizer() {
exists(string name | this = DataFlow::globalVarRef(name).getACall() |
name in ["encodeURI", "encodeURIComponent", "escape"]
)
this = DataFlow::globalVarRef(name).getACall() and
name in ["encodeURI", "encodeURIComponent", "escape"]
}
/**
* Holds if this URI encoding function properly encodes path separators,
* making it safe for request forgery prevention.
*/
predicate encodesPathSeparators() { name = "encodeURIComponent" }
}
/**

18
javascript/ql/test/query-tests/Security/CWE-918/RequestForgery.expected
View File

@@ -33,6 +33,8 @@
| serverSide.js:141:3:141:30 | axios.g ... ring()) | serverSide.js:139:17:139:29 | req.query.url | serverSide.js:141:13:141:29 | target.toString() | The $@ of this request depends on a $@. | serverSide.js:141:13:141:29 | target.toString() | URL | serverSide.js:139:17:139:29 | req.query.url | user-provided value |
| serverSide.js:142:3:142:19 | axios.get(target) | serverSide.js:139:17:139:29 | req.query.url | serverSide.js:142:13:142:18 | target | The $@ of this request depends on a $@. | serverSide.js:142:13:142:18 | target | URL | serverSide.js:139:17:139:29 | req.query.url | user-provided value |
| serverSide.js:143:3:143:24 | axios.g ... t.href) | serverSide.js:139:17:139:29 | req.query.url | serverSide.js:143:13:143:23 | target.href | The $@ of this request depends on a $@. | serverSide.js:143:13:143:23 | target.href | URL | serverSide.js:139:17:139:29 | req.query.url | user-provided value |
| serverSide.js:145:3:145:23 | axios.g ... dedUrl) | serverSide.js:139:17:139:29 | req.query.url | serverSide.js:145:13:145:22 | encodedUrl | The $@ of this request depends on a $@. | serverSide.js:145:13:145:22 | encodedUrl | URL | serverSide.js:139:17:139:29 | req.query.url | user-provided value |
| serverSide.js:147:3:147:23 | axios.g ... pedUrl) | serverSide.js:139:17:139:29 | req.query.url | serverSide.js:147:13:147:22 | escapedUrl | The $@ of this request depends on a $@. | serverSide.js:147:13:147:22 | escapedUrl | URL | serverSide.js:139:17:139:29 | req.query.url | user-provided value |
edges
| Request/app/api/proxy/route2.serverSide.ts:4:9:4:15 | { url } | Request/app/api/proxy/route2.serverSide.ts:4:9:4:34 | url | provenance | |
| Request/app/api/proxy/route2.serverSide.ts:4:9:4:34 | url | Request/app/api/proxy/route2.serverSide.ts:5:27:5:29 | url | provenance | |
@@ -110,6 +112,8 @@ edges
| serverSide.js:130:9:130:45 | myUrl | serverSide.js:131:15:131:19 | myUrl | provenance | |
| serverSide.js:130:37:130:43 | tainted | serverSide.js:130:9:130:45 | myUrl | provenance | |
| serverSide.js:139:9:139:29 | input | serverSide.js:140:26:140:30 | input | provenance | |
| serverSide.js:139:9:139:29 | input | serverSide.js:144:32:144:36 | input | provenance | |
| serverSide.js:139:9:139:29 | input | serverSide.js:146:29:146:33 | input | provenance | |
| serverSide.js:139:17:139:29 | req.query.url | serverSide.js:139:9:139:29 | input | provenance | |
| serverSide.js:140:9:140:31 | target | serverSide.js:141:13:141:18 | target | provenance | |
| serverSide.js:140:9:140:31 | target | serverSide.js:142:13:142:18 | target | provenance | |
@@ -118,6 +122,12 @@ edges
| serverSide.js:140:26:140:30 | input | serverSide.js:140:18:140:31 | new URL(input) | provenance | Config |
| serverSide.js:141:13:141:18 | target | serverSide.js:141:13:141:29 | target.toString() | provenance | |
| serverSide.js:143:13:143:18 | target | serverSide.js:143:13:143:23 | target.href | provenance | |
| serverSide.js:144:9:144:37 | encodedUrl | serverSide.js:145:13:145:22 | encodedUrl | provenance | |
| serverSide.js:144:22:144:37 | encodeURI(input) | serverSide.js:144:9:144:37 | encodedUrl | provenance | |
| serverSide.js:144:32:144:36 | input | serverSide.js:144:22:144:37 | encodeURI(input) | provenance | |
| serverSide.js:146:9:146:34 | escapedUrl | serverSide.js:147:13:147:22 | escapedUrl | provenance | |
| serverSide.js:146:22:146:34 | escape(input) | serverSide.js:146:9:146:34 | escapedUrl | provenance | |
| serverSide.js:146:29:146:33 | input | serverSide.js:146:22:146:34 | escape(input) | provenance | |
nodes
| Request/app/api/proxy/route2.serverSide.ts:4:9:4:15 | { url } | semmle.label | { url } |
| Request/app/api/proxy/route2.serverSide.ts:4:9:4:34 | url | semmle.label | url |
@@ -221,4 +231,12 @@ nodes
| serverSide.js:142:13:142:18 | target | semmle.label | target |
| serverSide.js:143:13:143:18 | target | semmle.label | target |
| serverSide.js:143:13:143:23 | target.href | semmle.label | target.href |
| serverSide.js:144:9:144:37 | encodedUrl | semmle.label | encodedUrl |
| serverSide.js:144:22:144:37 | encodeURI(input) | semmle.label | encodeURI(input) |
| serverSide.js:144:32:144:36 | input | semmle.label | input |
| serverSide.js:145:13:145:22 | encodedUrl | semmle.label | encodedUrl |
| serverSide.js:146:9:146:34 | escapedUrl | semmle.label | escapedUrl |
| serverSide.js:146:22:146:34 | escape(input) | semmle.label | escape(input) |
| serverSide.js:146:29:146:33 | input | semmle.label | input |
| serverSide.js:147:13:147:22 | escapedUrl | semmle.label | escapedUrl |
subpaths

4
javascript/ql/test/query-tests/Security/CWE-918/serverSide.js
View File

@@ -141,4 +141,8 @@ var server2 = http.createServer(function(req, res) {
axios.get(target.toString()); // $Alert[js/request-forgery]
axios.get(target); // $Alert[js/request-forgery]
axios.get(target.href); // $Alert[js/request-forgery]
const encodedUrl = encodeURI(input);
axios.get(encodedUrl); // $Alert[js/request-forgery]
const escapedUrl = escape(input);
axios.get(escapedUrl); // $Alert[js/request-forgery]
});