Skip to content

Commit 3cece50

Browse files
committed
add encodeURIComponent as a sanitizer for request-forgery
1 parent be8ef1b commit 3cece50

File tree

3 files changed

+11
-14
lines changed

3 files changed

+11
-14
lines changed

javascript/ql/lib/semmle/javascript/security/dataflow/RequestForgeryCustomizations.qll

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,4 +81,14 @@ module RequestForgery {
8181

8282
override string getKind() { result = "endpoint" }
8383
}
84+
85+
private import Xss as Xss
86+
87+
/**
88+
* A call to `encodeURI` or `encodeURIComponent`, viewed as a sanitizer for request forgery.
89+
* These calls will escape "/" to "%2F", which is not a problem for request forgery.
90+
* The result from calling `encodeURI` or `encodeURIComponent` is not a valid URL, and only makes sense
91+
* as a part of a URL.
92+
*/
93+
class UriEncodingSanitizer extends Sanitizer instanceof Xss::Shared::UriEncodingSanitizer { }
8494
}

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

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -101,12 +101,6 @@ nodes
101101
| serverSide.js:130:37:130:43 | tainted |
102102
| serverSide.js:131:15:131:19 | myUrl |
103103
| serverSide.js:131:15:131:19 | myUrl |
104-
| serverSide.js:133:9:133:72 | myEncodedUrl |
105-
| serverSide.js:133:24:133:72 | `${some ... nted)}` |
106-
| serverSide.js:133:44:133:70 | encodeU ... ainted) |
107-
| serverSide.js:133:63:133:69 | tainted |
108-
| serverSide.js:134:15:134:26 | myEncodedUrl |
109-
| serverSide.js:134:15:134:26 | myEncodedUrl |
110104
edges
111105
| serverSide.js:14:9:14:52 | tainted | serverSide.js:18:13:18:19 | tainted |
112106
| serverSide.js:14:9:14:52 | tainted | serverSide.js:18:13:18:19 | tainted |
@@ -194,7 +188,6 @@ edges
194188
| serverSide.js:123:9:123:52 | tainted | serverSide.js:127:14:127:20 | tainted |
195189
| serverSide.js:123:9:123:52 | tainted | serverSide.js:127:14:127:20 | tainted |
196190
| serverSide.js:123:9:123:52 | tainted | serverSide.js:130:37:130:43 | tainted |
197-
| serverSide.js:123:9:123:52 | tainted | serverSide.js:133:63:133:69 | tainted |
198191
| serverSide.js:123:19:123:42 | url.par ... , true) | serverSide.js:123:19:123:48 | url.par ... ).query |
199192
| serverSide.js:123:19:123:48 | url.par ... ).query | serverSide.js:123:19:123:52 | url.par ... ery.url |
200193
| serverSide.js:123:19:123:52 | url.par ... ery.url | serverSide.js:123:9:123:52 | tainted |
@@ -204,11 +197,6 @@ edges
204197
| serverSide.js:130:9:130:45 | myUrl | serverSide.js:131:15:131:19 | myUrl |
205198
| serverSide.js:130:17:130:45 | `${some ... inted}` | serverSide.js:130:9:130:45 | myUrl |
206199
| serverSide.js:130:37:130:43 | tainted | serverSide.js:130:17:130:45 | `${some ... inted}` |
207-
| serverSide.js:133:9:133:72 | myEncodedUrl | serverSide.js:134:15:134:26 | myEncodedUrl |
208-
| serverSide.js:133:9:133:72 | myEncodedUrl | serverSide.js:134:15:134:26 | myEncodedUrl |
209-
| serverSide.js:133:24:133:72 | `${some ... nted)}` | serverSide.js:133:9:133:72 | myEncodedUrl |
210-
| serverSide.js:133:44:133:70 | encodeU ... ainted) | serverSide.js:133:24:133:72 | `${some ... nted)}` |
211-
| serverSide.js:133:63:133:69 | tainted | serverSide.js:133:44:133:70 | encodeU ... ainted) |
212200
#select
213201
| serverSide.js:18:5:18:20 | request(tainted) | serverSide.js:14:29:14:35 | req.url | serverSide.js:18:13:18:19 | tainted | The $@ of this request depends on a $@. | serverSide.js:18:13:18:19 | tainted | URL | serverSide.js:14:29:14:35 | req.url | user-provided value |
214202
| serverSide.js:20:5:20:24 | request.get(tainted) | serverSide.js:14:29:14:35 | req.url | serverSide.js:20:17:20:23 | tainted | The $@ of this request depends on a $@. | serverSide.js:20:17:20:23 | tainted | URL | serverSide.js:14:29:14:35 | req.url | user-provided value |
@@ -234,4 +222,3 @@ edges
234222
| serverSide.js:117:20:117:30 | new ws(url) | serverSide.js:115:25:115:35 | request.url | serverSide.js:117:27:117:29 | url | The $@ of this request depends on a $@. | serverSide.js:117:27:117:29 | url | URL | serverSide.js:115:25:115:35 | request.url | user-provided value |
235223
| serverSide.js:125:5:128:6 | axios({ ... \\n }) | serverSide.js:123:29:123:35 | req.url | serverSide.js:127:14:127:20 | tainted | The $@ of this request depends on a $@. | serverSide.js:127:14:127:20 | tainted | URL | serverSide.js:123:29:123:35 | req.url | user-provided value |
236224
| serverSide.js:131:5:131:20 | axios.get(myUrl) | serverSide.js:123:29:123:35 | req.url | serverSide.js:131:15:131:19 | myUrl | The $@ of this request depends on a $@. | serverSide.js:131:15:131:19 | myUrl | URL | serverSide.js:123:29:123:35 | req.url | user-provided value |
237-
| serverSide.js:134:5:134:27 | axios.g ... dedUrl) | serverSide.js:123:29:123:35 | req.url | serverSide.js:134:15:134:26 | myEncodedUrl | The $@ of this request depends on a $@. | serverSide.js:134:15:134:26 | myEncodedUrl | URL | serverSide.js:123:29:123:35 | req.url | user-provided value |

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,5 +131,5 @@ var server2 = http.createServer(function(req, res) {
131131
axios.get(myUrl); // NOT OK
132132

133133
var myEncodedUrl = `${something}/bla/${encodeURIComponent(tainted)}`;
134-
axios.get(myEncodedUrl); // OK - but still flagged [INCONSISTENCY]
134+
axios.get(myEncodedUrl); // OK
135135
})

0 commit comments

Comments
 (0)