Skip to content

Commit 11f39a9

Browse files
authored
Merge pull request #4342 from erik-krogh/track-where-prop
Approved by asgerf
2 parents 060c19a + b8154d4 commit 11f39a9

File tree

4 files changed

+38
-1
lines changed

4 files changed

+38
-1
lines changed

javascript/ql/src/semmle/javascript/frameworks/NoSQL.qll

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,23 @@ module NoSQL {
1212
}
1313
}
1414

15+
/**
16+
* Gets a reference to an object where the "$where" property has been assigned to`rhs`.
17+
*/
18+
DataFlow::SourceNode getADollarWherePropertyValueSource(DataFlow::TypeTracker t, DataFlow::Node rhs) {
19+
t.start() and
20+
rhs = result.getAPropertyWrite("$where").getRhs()
21+
or
22+
exists(DataFlow::TypeTracker t2 |
23+
result = getADollarWherePropertyValueSource(t2, rhs).track(t2, t)
24+
)
25+
}
26+
1527
/**
1628
* Gets the value of a `$where` property of an object that flows to `n`.
1729
*/
1830
private DataFlow::Node getADollarWherePropertyValue(DataFlow::Node n) {
19-
result = n.getALocalSource().getAPropertyWrite("$where").getRhs()
31+
getADollarWherePropertyValueSource(DataFlow::TypeTracker::end(), result).flowsTo(n)
2032
}
2133

2234
/**

javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/CodeInjection.expected

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,11 @@ nodes
88
| NoSQLCodeInjection.js:19:36:19:43 | req.body |
99
| NoSQLCodeInjection.js:19:36:19:43 | req.body |
1010
| NoSQLCodeInjection.js:19:36:19:48 | req.body.name |
11+
| NoSQLCodeInjection.js:22:24:22:48 | "name = ... dy.name |
12+
| NoSQLCodeInjection.js:22:24:22:48 | "name = ... dy.name |
13+
| NoSQLCodeInjection.js:22:36:22:43 | req.body |
14+
| NoSQLCodeInjection.js:22:36:22:43 | req.body |
15+
| NoSQLCodeInjection.js:22:36:22:48 | req.body.name |
1116
| angularjs.js:10:22:10:29 | location |
1217
| angularjs.js:10:22:10:29 | location |
1318
| angularjs.js:10:22:10:36 | location.search |
@@ -152,6 +157,10 @@ edges
152157
| NoSQLCodeInjection.js:19:36:19:43 | req.body | NoSQLCodeInjection.js:19:36:19:48 | req.body.name |
153158
| NoSQLCodeInjection.js:19:36:19:48 | req.body.name | NoSQLCodeInjection.js:19:24:19:48 | "name = ... dy.name |
154159
| NoSQLCodeInjection.js:19:36:19:48 | req.body.name | NoSQLCodeInjection.js:19:24:19:48 | "name = ... dy.name |
160+
| NoSQLCodeInjection.js:22:36:22:43 | req.body | NoSQLCodeInjection.js:22:36:22:48 | req.body.name |
161+
| NoSQLCodeInjection.js:22:36:22:43 | req.body | NoSQLCodeInjection.js:22:36:22:48 | req.body.name |
162+
| NoSQLCodeInjection.js:22:36:22:48 | req.body.name | NoSQLCodeInjection.js:22:24:22:48 | "name = ... dy.name |
163+
| NoSQLCodeInjection.js:22:36:22:48 | req.body.name | NoSQLCodeInjection.js:22:24:22:48 | "name = ... dy.name |
155164
| angularjs.js:10:22:10:29 | location | angularjs.js:10:22:10:36 | location.search |
156165
| angularjs.js:10:22:10:29 | location | angularjs.js:10:22:10:36 | location.search |
157166
| angularjs.js:10:22:10:29 | location | angularjs.js:10:22:10:36 | location.search |
@@ -275,6 +284,7 @@ edges
275284
#select
276285
| NoSQLCodeInjection.js:18:24:18:37 | req.body.query | NoSQLCodeInjection.js:18:24:18:31 | req.body | NoSQLCodeInjection.js:18:24:18:37 | req.body.query | $@ flows to here and is interpreted as code. | NoSQLCodeInjection.js:18:24:18:31 | req.body | User-provided value |
277286
| NoSQLCodeInjection.js:19:24:19:48 | "name = ... dy.name | NoSQLCodeInjection.js:19:36:19:43 | req.body | NoSQLCodeInjection.js:19:24:19:48 | "name = ... dy.name | $@ flows to here and is interpreted as code. | NoSQLCodeInjection.js:19:36:19:43 | req.body | User-provided value |
287+
| NoSQLCodeInjection.js:22:24:22:48 | "name = ... dy.name | NoSQLCodeInjection.js:22:36:22:43 | req.body | NoSQLCodeInjection.js:22:24:22:48 | "name = ... dy.name | $@ flows to here and is interpreted as code. | NoSQLCodeInjection.js:22:36:22:43 | req.body | User-provided value |
278288
| angularjs.js:10:22:10:36 | location.search | angularjs.js:10:22:10:29 | location | angularjs.js:10:22:10:36 | location.search | $@ flows to here and is interpreted as code. | angularjs.js:10:22:10:29 | location | User-provided value |
279289
| angularjs.js:13:23:13:37 | location.search | angularjs.js:13:23:13:30 | location | angularjs.js:13:23:13:37 | location.search | $@ flows to here and is interpreted as code. | angularjs.js:13:23:13:30 | location | User-provided value |
280290
| angularjs.js:16:28:16:42 | location.search | angularjs.js:16:28:16:35 | location | angularjs.js:16:28:16:42 | location.search | $@ flows to here and is interpreted as code. | angularjs.js:16:28:16:35 | location | User-provided value |

javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/HeuristicSourceCodeInjection.expected

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,11 @@ nodes
88
| NoSQLCodeInjection.js:19:36:19:43 | req.body |
99
| NoSQLCodeInjection.js:19:36:19:43 | req.body |
1010
| NoSQLCodeInjection.js:19:36:19:48 | req.body.name |
11+
| NoSQLCodeInjection.js:22:24:22:48 | "name = ... dy.name |
12+
| NoSQLCodeInjection.js:22:24:22:48 | "name = ... dy.name |
13+
| NoSQLCodeInjection.js:22:36:22:43 | req.body |
14+
| NoSQLCodeInjection.js:22:36:22:43 | req.body |
15+
| NoSQLCodeInjection.js:22:36:22:48 | req.body.name |
1116
| angularjs.js:10:22:10:29 | location |
1217
| angularjs.js:10:22:10:29 | location |
1318
| angularjs.js:10:22:10:36 | location.search |
@@ -156,6 +161,10 @@ edges
156161
| NoSQLCodeInjection.js:19:36:19:43 | req.body | NoSQLCodeInjection.js:19:36:19:48 | req.body.name |
157162
| NoSQLCodeInjection.js:19:36:19:48 | req.body.name | NoSQLCodeInjection.js:19:24:19:48 | "name = ... dy.name |
158163
| NoSQLCodeInjection.js:19:36:19:48 | req.body.name | NoSQLCodeInjection.js:19:24:19:48 | "name = ... dy.name |
164+
| NoSQLCodeInjection.js:22:36:22:43 | req.body | NoSQLCodeInjection.js:22:36:22:48 | req.body.name |
165+
| NoSQLCodeInjection.js:22:36:22:43 | req.body | NoSQLCodeInjection.js:22:36:22:48 | req.body.name |
166+
| NoSQLCodeInjection.js:22:36:22:48 | req.body.name | NoSQLCodeInjection.js:22:24:22:48 | "name = ... dy.name |
167+
| NoSQLCodeInjection.js:22:36:22:48 | req.body.name | NoSQLCodeInjection.js:22:24:22:48 | "name = ... dy.name |
159168
| angularjs.js:10:22:10:29 | location | angularjs.js:10:22:10:36 | location.search |
160169
| angularjs.js:10:22:10:29 | location | angularjs.js:10:22:10:36 | location.search |
161170
| angularjs.js:10:22:10:29 | location | angularjs.js:10:22:10:36 | location.search |

javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/NoSQLCodeInjection.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,5 +17,11 @@ app.post("/documents/find", (req, res) => {
1717
doc.find(query); // NOT OK, but that is flagged by js/sql-injection [INCONSISTENCY]
1818
doc.find({ $where: req.body.query }); // NOT OK
1919
doc.find({ $where: "name = " + req.body.name }); // NOT OK
20+
21+
function mkWhereObj() {
22+
return { $where: "name = " + req.body.name }; // NOT OK
23+
}
24+
25+
doc.find(mkWhereObj()); // the alert location is in mkWhereObj.
2026
});
2127
});

0 commit comments

Comments
 (0)