Skip to content

Commit 7a2faa0

Browse files
esbenaesbena
committed
JS: add additional mongoose and mongodb js/nosql-injection sinks
1 parent b6c616e commit 7a2faa0

3 files changed

Lines changed: 362 additions & 20 deletions

File tree

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

Lines changed: 280 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -120,41 +120,69 @@ private module MongoDB {
120120

121121
QueryCall() {
122122
exists(string m | this = getACollection().getAMethodCall(m) |
123-
m = "aggregate" and queryArgIdx = 0
123+
CollectionMethodSignatures::interpretsArgumentAsQuery(m, queryArgIdx)
124+
)
125+
}
126+
127+
override DataFlow::Node getAQueryArgument() { result = getArgument(queryArgIdx) }
128+
}
129+
130+
/**
131+
* Provides signatures for the Collection methods.
132+
*/
133+
module CollectionMethodSignatures {
134+
/**
135+
* Holds if Collection method `name` interprets parameter `n` as a query.
136+
*/
137+
predicate interpretsArgumentAsQuery(string name, int n) {
138+
// FilterQuery
139+
(
140+
name = "aggregate" and n = 0
141+
or
142+
name = "count" and n = 0
124143
or
125-
m = "count" and queryArgIdx = 0
144+
name = "countDocuments" and n = 0
126145
or
127-
m = "deleteMany" and queryArgIdx = 0
146+
name = "deleteMany" and n = 0
128147
or
129-
m = "deleteOne" and queryArgIdx = 0
148+
name = "deleteOne" and n = 0
130149
or
131-
m = "distinct" and queryArgIdx = 1
150+
name = "distinct" and n = 1
132151
or
133-
m = "find" and queryArgIdx = 0
152+
name = "find" and n = 0
134153
or
135-
m = "findOne" and queryArgIdx = 0
154+
name = "findOne" and n = 0
136155
or
137-
m = "findOneAndDelete" and queryArgIdx = 0
156+
name = "findOneAndDelete" and n = 0
138157
or
139-
m = "findOneAndRemove" and queryArgIdx = 0
158+
name = "findOneAndRemove" and n = 0
140159
or
141-
m = "findOneAndDelete" and queryArgIdx = 0
160+
name = "findOneAndReplace" and n = 0
142161
or
143-
m = "findOneAndUpdate" and queryArgIdx = 0
162+
name = "findOneAndUpdate" and n = 0
144163
or
145-
m = "remove" and queryArgIdx = 0
164+
name = "remove" and n = 0
146165
or
147-
m = "replaceOne" and queryArgIdx = 0
166+
name = "replaceOne" and n = 0
148167
or
149-
m = "update" and queryArgIdx = 0
168+
name = "update" and n = 0
150169
or
151-
m = "updateMany" and queryArgIdx = 0
170+
name = "updateMany" and n = 0
152171
or
153-
m = "updateOne" and queryArgIdx = 0
172+
name = "updateOne" and n = 0
173+
)
174+
or
175+
// UpdateQuery
176+
(
177+
name = "findOneAndUpdate" and n = 1
178+
or
179+
name = "update" and n = 1
180+
or
181+
name = "updateMany" and n = 1
182+
or
183+
name = "updateOne" and n = 1
154184
)
155185
}
156-
157-
override DataFlow::Node getAQueryArgument() { result = getArgument(queryArgIdx) }
158186
}
159187

160188
/**
@@ -184,8 +212,221 @@ private module Mongoose {
184212
/**
185213
* A Mongoose collection object.
186214
*/
187-
class Model extends MongoDB::Collection {
215+
class Model extends DataFlow::SourceNode {
188216
Model() { this = getAMongooseInstance().getAMemberCall("model") }
217+
218+
private DataFlow::SourceNode ref(DataFlow::TypeTracker t) {
219+
result = this and
220+
t.start()
221+
or
222+
exists(DataFlow::TypeTracker t2 | result = ref(t2).track(t2, t))
223+
}
224+
225+
DataFlow::SourceNode ref() { result = ref(DataFlow::TypeTracker::end()) }
226+
}
227+
228+
/**
229+
* Provides signatures for the Model methods.
230+
*/
231+
module ModelMethodSignatures {
232+
/**
233+
* Holds if Model method `name` interprets parameter `n` as a query.
234+
*/
235+
predicate interpretsArgumentAsQuery(string name, int n) {
236+
// implement lots of the MongoDB collection interface
237+
MongoDB::CollectionMethodSignatures::interpretsArgumentAsQuery(name, n)
238+
or
239+
name = "findByIdAndUpdate" and n = 1
240+
}
241+
242+
/**
243+
* Holds if Model method `name` returns a Query.
244+
*/
245+
predicate returnsQuery(string name) {
246+
name = "$where" or
247+
name = "count" or
248+
name = "countDocuments" or
249+
name = "deleteMany" or
250+
name = "deleteOne" or
251+
name = "find" or
252+
name = "findById" or
253+
name = "findByIdAndDelete" or
254+
name = "findByIdAndRemove" or
255+
name = "findByIdAndUpdate" or
256+
name = "findOne" or
257+
name = "findOneAndDelete" or
258+
name = "findOneAndRemove" or
259+
name = "findOneAndReplace" or
260+
name = "findOneAndUpdate" or
261+
name = "geosearch" or
262+
name = "replaceOne" or
263+
name = "update" or
264+
name = "updateMany" or
265+
name = "updateOne" or
266+
name = "where"
267+
}
268+
}
269+
270+
/**
271+
* Provides signatures for the Query methods.
272+
*/
273+
module QueryMethodSignatures {
274+
/**
275+
* Holds if Query method `name` interprets parameter `n` as a query.
276+
*/
277+
predicate interpretsArgumentAsQuery(string name, int n) {
278+
n = 0 and
279+
(
280+
name = "and" or
281+
name = "count" or
282+
name = "countDocuments" or
283+
name = "deleteMany" or
284+
name = "deleteOne" or
285+
name = "elemMatch" or
286+
name = "find" or
287+
name = "findOne" or
288+
name = "findOneAndDelete" or
289+
name = "findOneAndRemove" or
290+
name = "findOneAndReplace" or
291+
name = "findOneAndUpdate" or
292+
name = "merge" or
293+
name = "nor" or
294+
name = "or" or
295+
name = "remove" or
296+
name = "replaceOne" or
297+
name = "setQuery" or
298+
name = "setUpdate" or
299+
name = "update" or
300+
name = "updateMany" or
301+
name = "updateOne" or
302+
name = "where"
303+
)
304+
or
305+
n = 1 and
306+
(
307+
name = "distinct" or
308+
name = "findOneAndUpdate" or
309+
name = "update" or
310+
name = "updateMany" or
311+
name = "updateOne"
312+
)
313+
}
314+
315+
/**
316+
* Holds if Query method `name` returns a Query.
317+
*/
318+
predicate returnsQuery(string name) {
319+
name = "$where" or
320+
name = "J" or
321+
name = "all" or
322+
name = "and" or
323+
name = "batchsize" or
324+
name = "box" or
325+
name = "center" or
326+
name = "centerSphere" or
327+
name = "circle" or
328+
name = "collation" or
329+
name = "comment" or
330+
name = "count" or
331+
name = "countDocuments" or
332+
name = "distinct" or
333+
name = "elemMatch" or
334+
name = "equals" or
335+
name = "error" or
336+
name = "estimatedDocumentCount" or
337+
name = "exists" or
338+
name = "explain" or
339+
name = "find" or
340+
name = "findById" or
341+
name = "findOne" or
342+
name = "findOneAndRemove" or
343+
name = "findOneAndUpdate" or
344+
name = "geometry" or
345+
name = "get" or
346+
name = "gt" or
347+
name = "gte" or
348+
name = "hint" or
349+
name = "in" or
350+
name = "intersects" or
351+
name = "lean" or
352+
name = "limit" or
353+
name = "lt" or
354+
name = "lte" or
355+
name = "map" or
356+
name = "map" or
357+
name = "maxDistance" or
358+
name = "maxTimeMS" or
359+
name = "maxscan" or
360+
name = "mod" or
361+
name = "ne" or
362+
name = "near" or
363+
name = "nearSphere" or
364+
name = "nin" or
365+
name = "or" or
366+
name = "orFail" or
367+
name = "polygon" or
368+
name = "populate" or
369+
name = "read" or
370+
name = "readConcern" or
371+
name = "regexp" or
372+
name = "remove" or
373+
name = "select" or
374+
name = "session" or
375+
name = "set" or
376+
name = "setOptions" or
377+
name = "setQuery" or
378+
name = "setUpdate" or
379+
name = "size" or
380+
name = "skip" or
381+
name = "slaveOk" or
382+
name = "slice" or
383+
name = "snapshot" or
384+
name = "sort" or
385+
name = "update" or
386+
name = "w" or
387+
name = "where" or
388+
name = "within" or
389+
name = "wtimeout"
390+
}
391+
}
392+
393+
/**
394+
* A Mongoose query object as a result of a Model method call.
395+
*/
396+
private class QueryFromModel extends DataFlow::MethodCallNode {
397+
QueryFromModel() {
398+
exists(string name, Model m |
399+
ModelMethodSignatures::returnsQuery(name) and
400+
m.ref().getAMethodCall(name) = this
401+
)
402+
}
403+
}
404+
405+
/**
406+
* A Mongoose query object as a result of a Query constructor invocation.
407+
*/
408+
private class QueryFromConstructor extends DataFlow::NewNode {
409+
QueryFromConstructor() {
410+
this = getAMongooseInstance().getAPropertyRead("Query").getAnInstantiation()
411+
}
412+
}
413+
414+
/**
415+
* Gets a data flow node referring to a Mongoose query object.
416+
*/
417+
private DataFlow::SourceNode getAQuery(DataFlow::TypeTracker t) {
418+
(
419+
result instanceof QueryFromConstructor or
420+
result instanceof QueryFromModel
421+
) and
422+
t.start()
423+
or
424+
exists(DataFlow::TypeTracker t2, DataFlow::SourceNode succ | succ = getAQuery(t2) |
425+
result = succ.track(t2, t)
426+
or
427+
result = succ.getAMethodCall(any(string name | QueryMethodSignatures::returnsQuery(name))) and
428+
t = t2
429+
)
189430
}
190431

191432
/**
@@ -204,4 +445,24 @@ private module Mongoose {
204445

205446
override string getCredentialsKind() { result = kind }
206447
}
448+
449+
/**
450+
* An expression that is interpreted as a (part of a) MongoDB query.
451+
*/
452+
class MongoDBQueryPart extends NoSQL::Query {
453+
MongoDBQueryPart() {
454+
exists(Model m, DataFlow::MethodCallNode mcn, string method, int n |
455+
ModelMethodSignatures::interpretsArgumentAsQuery(method, n) and
456+
mcn = m.ref().getAMethodCall(method) and
457+
this = mcn.getArgument(n).asExpr()
458+
)
459+
or
460+
this = any(QueryFromConstructor c).getArgument(2).asExpr()
461+
or
462+
exists(string method, int n | QueryMethodSignatures::interpretsArgumentAsQuery(method, n) |
463+
this =
464+
getAQuery(DataFlow::TypeTracker::end()).getAMethodCall(method).getArgument(n).asExpr()
465+
)
466+
}
467+
}
207468
}

0 commit comments

Comments
 (0)