Skip to content

Commit be2fe6e

Browse files
authored
Merge pull request github#5630 from erik-krogh/urlStep
Approved by esbena
2 parents 8d2768b + 99dd533 commit be2fe6e

File tree

4 files changed

+34
-0
lines changed

4 files changed

+34
-0
lines changed

javascript/ql/src/semmle/javascript/security/dataflow/RequestForgery.qll

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,5 +31,9 @@ module RequestForgery {
3131
override predicate isSanitizerEdge(DataFlow::Node source, DataFlow::Node sink) {
3232
sanitizingPrefixEdge(source, sink)
3333
}
34+
35+
override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
36+
isAdditionalRequestForgeryStep(pred, succ)
37+
}
3438
}
3539
}

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

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

6060
override string getKind() { result = kind }
6161
}
62+
63+
/**
64+
* Holds if there is a taint step from `pred` to `succ` for request forgery.
65+
*/
66+
predicate isAdditionalRequestForgeryStep(DataFlow::Node pred, DataFlow::Node succ) {
67+
exists(DataFlow::NewNode url | url = DataFlow::globalVarRef("URL").getAnInstantiation() |
68+
succ = url and
69+
pred = url.getArgument(0)
70+
)
71+
}
6272
}

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,12 @@ nodes
8282
| tst.js:108:17:108:27 | request.url |
8383
| tst.js:109:27:109:29 | url |
8484
| tst.js:109:27:109:29 | url |
85+
| tst.js:115:11:115:42 | url |
86+
| tst.js:115:17:115:42 | new URL ... , base) |
87+
| tst.js:115:25:115:35 | request.url |
88+
| tst.js:115:25:115:35 | request.url |
89+
| tst.js:117:27:117:29 | url |
90+
| tst.js:117:27:117:29 | url |
8591
edges
8692
| tst.js:14:9:14:52 | tainted | tst.js:18:13:18:19 | tainted |
8793
| tst.js:14:9:14:52 | tainted | tst.js:18:13:18:19 | tainted |
@@ -161,6 +167,11 @@ edges
161167
| tst.js:108:11:108:27 | url | tst.js:109:27:109:29 | url |
162168
| tst.js:108:17:108:27 | request.url | tst.js:108:11:108:27 | url |
163169
| tst.js:108:17:108:27 | request.url | tst.js:108:11:108:27 | url |
170+
| tst.js:115:11:115:42 | url | tst.js:117:27:117:29 | url |
171+
| tst.js:115:11:115:42 | url | tst.js:117:27:117:29 | url |
172+
| tst.js:115:17:115:42 | new URL ... , base) | tst.js:115:11:115:42 | url |
173+
| tst.js:115:25:115:35 | request.url | tst.js:115:17:115:42 | new URL ... , base) |
174+
| tst.js:115:25:115:35 | request.url | tst.js:115:17:115:42 | new URL ... , base) |
164175
#select
165176
| tst.js:18:5:18:20 | request(tainted) | tst.js:14:29:14:35 | req.url | tst.js:18:13:18:19 | tainted | The $@ of this request depends on $@. | tst.js:18:13:18:19 | tainted | URL | tst.js:14:29:14:35 | req.url | a user-provided value |
166177
| tst.js:20:5:20:24 | request.get(tainted) | tst.js:14:29:14:35 | req.url | tst.js:20:17:20:23 | tainted | The $@ of this request depends on $@. | tst.js:20:17:20:23 | tainted | URL | tst.js:14:29:14:35 | req.url | a user-provided value |
@@ -183,3 +194,4 @@ edges
183194
| tst.js:92:5:92:33 | JSDOM.f ... ms.foo) | tst.js:92:19:92:28 | ctx.params | tst.js:92:19:92:32 | ctx.params.foo | The $@ of this request depends on $@. | tst.js:92:19:92:32 | ctx.params.foo | URL | tst.js:92:19:92:28 | ctx.params | a user-provided value |
184195
| tst.js:100:5:100:26 | new Web ... ainted) | tst.js:98:29:98:35 | req.url | tst.js:100:19:100:25 | tainted | The $@ of this request depends on $@. | tst.js:100:19:100:25 | tainted | URL | tst.js:98:29:98:35 | req.url | a user-provided value |
185196
| tst.js:109:20:109:30 | new ws(url) | tst.js:108:17:108:27 | request.url | tst.js:109:27:109:29 | url | The $@ of this request depends on $@. | tst.js:109:27:109:29 | url | URL | tst.js:108:17:108:27 | request.url | a user-provided value |
197+
| tst.js:117:20:117:30 | new ws(url) | tst.js:115:25:115:35 | request.url | tst.js:117:27:117:29 | url | The $@ of this request depends on $@. | tst.js:117:27:117:29 | url | URL | tst.js:115:25:115:35 | request.url | a user-provided value |

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,3 +109,11 @@ new ws.Server({ port: 8080 }).on('connection', function(socket, request) {
109109
const socket = new ws(url);
110110
});
111111
});
112+
113+
new ws.Server({ port: 8080 }).on('connection', function (socket, request) {
114+
socket.on('message', function (message) {
115+
const url = new URL(request.url, base);
116+
const target = new URL(url.pathname, base);
117+
const socket = new ws(url);
118+
});
119+
});

0 commit comments

Comments
 (0)