Skip to content

Commit 70cf1a5

Browse files
committed
Now catches usage of RegExp. after matchAll usage.
1 parent c2baf0b commit 70cf1a5

File tree

3 files changed

+16
-2
lines changed

3 files changed

+16
-2
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -917,7 +917,7 @@ module TaintTracking {
917917
*/
918918
private ControlFlowNode getACaptureSetter(DataFlow::Node input) {
919919
exists(DataFlow::MethodCallNode call | result = call.asExpr() |
920-
call.getMethodName() = ["search", "replace", "replaceAll", "match"] and
920+
call.getMethodName() = ["search", "replace", "replaceAll", "match", "matchAll"] and
921921
input = call.getReceiver()
922922
or
923923
call.getMethodName() = ["test", "exec"] and input = call.getArgument(0)

javascript/ql/test/query-tests/Security/CWE-117/LogInjection.expected

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,13 @@ nodes
106106
| logInjectionBad.js:123:20:123:46 | usernam ... */g)[0] |
107107
| logInjectionBad.js:124:17:124:24 | otherStr |
108108
| logInjectionBad.js:124:17:124:24 | otherStr |
109+
| logInjectionBad.js:128:20:128:43 | url.par ... , true) |
110+
| logInjectionBad.js:128:20:128:49 | url.par ... ).query |
111+
| logInjectionBad.js:128:20:128:58 | url.par ... sername |
112+
| logInjectionBad.js:128:30:128:36 | req.url |
113+
| logInjectionBad.js:128:30:128:36 | req.url |
114+
| logInjectionBad.js:129:42:129:50 | RegExp.$1 |
115+
| logInjectionBad.js:129:42:129:50 | RegExp.$1 |
109116
edges
110117
| logInjectionBad.js:19:9:19:36 | q | logInjectionBad.js:20:20:20:20 | q |
111118
| logInjectionBad.js:19:13:19:36 | url.par ... , true) | logInjectionBad.js:19:9:19:36 | q |
@@ -209,6 +216,12 @@ edges
209216
| logInjectionBad.js:123:20:123:27 | username | logInjectionBad.js:123:20:123:43 | usernam ... (/.*/g) |
210217
| logInjectionBad.js:123:20:123:43 | usernam ... (/.*/g) | logInjectionBad.js:123:20:123:46 | usernam ... */g)[0] |
211218
| logInjectionBad.js:123:20:123:46 | usernam ... */g)[0] | logInjectionBad.js:123:9:123:46 | otherStr |
219+
| logInjectionBad.js:128:20:128:43 | url.par ... , true) | logInjectionBad.js:128:20:128:49 | url.par ... ).query |
220+
| logInjectionBad.js:128:20:128:49 | url.par ... ).query | logInjectionBad.js:128:20:128:58 | url.par ... sername |
221+
| logInjectionBad.js:128:20:128:58 | url.par ... sername | logInjectionBad.js:129:42:129:50 | RegExp.$1 |
222+
| logInjectionBad.js:128:20:128:58 | url.par ... sername | logInjectionBad.js:129:42:129:50 | RegExp.$1 |
223+
| logInjectionBad.js:128:30:128:36 | req.url | logInjectionBad.js:128:20:128:43 | url.par ... , true) |
224+
| logInjectionBad.js:128:30:128:36 | req.url | logInjectionBad.js:128:20:128:43 | url.par ... , true) |
212225
#select
213226
| logInjectionBad.js:22:18:22:43 | `[INFO] ... rname}` | logInjectionBad.js:19:23:19:29 | req.url | logInjectionBad.js:22:18:22:43 | `[INFO] ... rname}` | Log entry depends on a $@. | logInjectionBad.js:19:23:19:29 | req.url | user-provided value |
214227
| logInjectionBad.js:23:37:23:44 | username | logInjectionBad.js:19:23:19:29 | req.url | logInjectionBad.js:23:37:23:44 | username | Log entry depends on a $@. | logInjectionBad.js:19:23:19:29 | req.url | user-provided value |
@@ -232,3 +245,4 @@ edges
232245
| logInjectionBad.js:99:26:99:33 | username | logInjectionBad.js:72:23:72:29 | req.url | logInjectionBad.js:99:26:99:33 | username | Log entry depends on a $@. | logInjectionBad.js:72:23:72:29 | req.url | user-provided value |
233246
| logInjectionBad.js:113:37:113:44 | username | logInjectionBad.js:72:23:72:29 | req.url | logInjectionBad.js:113:37:113:44 | username | Log entry depends on a $@. | logInjectionBad.js:72:23:72:29 | req.url | user-provided value |
234247
| logInjectionBad.js:124:17:124:24 | otherStr | logInjectionBad.js:122:30:122:36 | req.url | logInjectionBad.js:124:17:124:24 | otherStr | Log entry depends on a $@. | logInjectionBad.js:122:30:122:36 | req.url | user-provided value |
248+
| logInjectionBad.js:129:42:129:50 | RegExp.$1 | logInjectionBad.js:128:30:128:36 | req.url | logInjectionBad.js:129:42:129:50 | RegExp.$1 | Log entry depends on a $@. | logInjectionBad.js:128:30:128:36 | req.url | user-provided value |

javascript/ql/test/query-tests/Security/CWE-117/logInjectionBad.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,6 @@ const serverMatchAll = http.createServer((req, res) => {
125125
});
126126

127127
const serverMatchAl2l = http.createServer((req, res) => {
128-
const result = url.parse(req.url, true).query.username.matchAll(/(\d+)/g); // BAD - match is marked as vulnerable, while matchAll is not.
128+
const result = url.parse(req.url, true).query.username.matchAll(/(\d+)/g); // BAD
129129
console.log("First captured group:", RegExp.$1);
130130
});

0 commit comments

Comments
 (0)