Skip to content

Commit 9d9926f

Browse files
committed
JS: model Mongoose Document for additional js/nosql-injection sinks
1 parent 55ab519 commit 9d9926f

2 files changed

Lines changed: 210 additions & 0 deletions

File tree

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

Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,16 @@ private module Mongoose {
277277
name = "updateOne" or
278278
name = "where"
279279
}
280+
281+
/**
282+
* Holds if Model method `name` returns one or more documents, the
283+
* documents are wrapped in an array if `asArray` is true.
284+
*/
285+
predicate returnsDocument(string name, boolean asArray) {
286+
asArray = false and name = "findOne"
287+
or
288+
asArray = true and name = "find"
289+
}
280290
}
281291
}
282292

@@ -296,6 +306,18 @@ private module Mongoose {
296306
}
297307
}
298308

309+
/**
310+
* A Mongoose query object as a result of a Document method call.
311+
*/
312+
private class QueryFromDocument extends DataFlow::MethodCallNode {
313+
QueryFromDocument() {
314+
exists(string name |
315+
Document::MethodSignatures::returnsQuery(name) and
316+
Document::ref().getAMethodCall(name) = this
317+
)
318+
}
319+
}
320+
299321
/**
300322
* A Mongoose query object as a result of a Query constructor invocation.
301323
*/
@@ -312,6 +334,7 @@ private module Mongoose {
312334
(
313335
result instanceof QueryFromConstructor or
314336
result instanceof QueryFromModel or
337+
result instanceof QueryFromDocument or
315338
result.hasUnderlyingType("mongoose", "Query")
316339
) and
317340
t.start()
@@ -450,6 +473,138 @@ private module Mongoose {
450473
name = "within" or
451474
name = "wtimeout"
452475
}
476+
477+
/**
478+
* Holds if Query method `name` returns one or more documents, the
479+
* documents are wrapped in an array if `asArray` is true.
480+
*/
481+
predicate returnsDocument(string name, boolean asArray) {
482+
asArray = false and name = "findOne"
483+
or
484+
asArray = true and name = "find"
485+
}
486+
}
487+
}
488+
489+
/**
490+
* Provides classes modeling the Mongoose Document class
491+
*/
492+
module Document {
493+
/**
494+
* A Mongoose Document that is retrieved from the backing database.
495+
*/
496+
class RetrievedDocument extends DataFlow::SourceNode {
497+
RetrievedDocument() {
498+
exists(boolean asArray, DataFlow::ParameterNode param |
499+
exists(DataFlow::SourceNode base, DataFlow::MethodCallNode call, string name |
500+
base = Query::ref() and Query::MethodSignatures::returnsDocument(name, asArray)
501+
or
502+
base = Model::ref() and Model::MethodSignatures::returnsDocument(name, asArray)
503+
or
504+
base = ref() and MethodSignatures::returnsDocument(name, asArray)
505+
|
506+
call = base.getAMethodCall(name) and
507+
param = call.getCallback(call.getNumArgument() - 1).getParameter(1)
508+
)
509+
or
510+
exists(
511+
DataFlow::SourceNode base, DataFlow::MethodCallNode call, string executor,
512+
int paramIndex
513+
|
514+
executor = "then" and paramIndex = 0
515+
or
516+
executor = "exec" and paramIndex = 1
517+
|
518+
base = Query::ref() and
519+
call = base.getAMethodCall(executor) and
520+
param = call.getCallback(0).getParameter(paramIndex) and
521+
exists(DataFlow::MethodCallNode pred |
522+
// limitation: look at the previous method call
523+
Query::MethodSignatures::returnsDocument(pred.getMethodName(), asArray) and
524+
pred.getAMethodCall() = call
525+
)
526+
)
527+
|
528+
asArray = false and this = param
529+
or
530+
asArray = true and
531+
exists(DataFlow::PropRead access |
532+
// limitation: look for direct accesses
533+
access = param.getAPropertyRead() and
534+
not exists(access.getPropertyName()) and
535+
this = access
536+
)
537+
)
538+
}
539+
}
540+
541+
/**
542+
* Gets a data flow node referring to a Mongoose Document object.
543+
*/
544+
private DataFlow::SourceNode ref(DataFlow::TypeTracker t) {
545+
(
546+
result instanceof RetrievedDocument or
547+
result.hasUnderlyingType("mongoose", "Document")
548+
) and
549+
t.start()
550+
or
551+
exists(DataFlow::TypeTracker t2, DataFlow::SourceNode succ | succ = ref(t2) |
552+
result = succ.track(t2, t)
553+
or
554+
result =
555+
succ.getAMethodCall(any(string name | MethodSignatures::returnsDocument(name, true))) and
556+
t = t2.continue()
557+
)
558+
}
559+
560+
/**
561+
* Gets a data flow node referring to a Mongoose Document object.
562+
*/
563+
DataFlow::SourceNode ref() { result = ref(DataFlow::TypeTracker::end()) }
564+
565+
module MethodSignatures {
566+
/**
567+
* Holds if Document method `name` returns a Query.
568+
*/
569+
predicate returnsQuery(string name) {
570+
// Documents are subtypes of Models
571+
Model::MethodSignatures::returnsQuery(name) or
572+
name = "replaceOne" or
573+
name = "update" or
574+
name = "updateOne"
575+
}
576+
577+
/**
578+
* Holds if Document method `name` interprets parameter `n` as a query.
579+
*/
580+
predicate interpretsArgumentAsQuery(string name, int n) {
581+
// Documents are subtypes of Models
582+
Model::MethodSignatures::interpretsArgumentAsQuery(name, n)
583+
or
584+
n = 0 and
585+
(
586+
name = "replaceOne" or
587+
name = "update" or
588+
name = "updateOne"
589+
)
590+
}
591+
592+
/**
593+
* Holds if Document method `name` returns one or more documents, the
594+
* documents are wrapped in an array if `asArray` is true.
595+
*/
596+
predicate returnsDocument(string name, boolean asArray) {
597+
// Documents are subtypes of Models
598+
Model::MethodSignatures::returnsDocument(name, asArray)
599+
or
600+
asArray = false and
601+
(
602+
name = "depopulate" or
603+
name = "init" or
604+
name = "populate" or
605+
name = "overwrite"
606+
)
607+
}
453608
}
454609
}
455610

@@ -486,6 +641,12 @@ private module Mongoose {
486641
exists(string method, int n | Query::MethodSignatures::interpretsArgumentAsQuery(method, n) |
487642
this = Query::ref().getAMethodCall(method).getArgument(n).asExpr()
488643
)
644+
or
645+
exists(DataFlow::MethodCallNode mcn, string method, int n |
646+
Document::MethodSignatures::interpretsArgumentAsQuery(method, n) and
647+
mcn = Document::ref().getAMethodCall(method) and
648+
this = mcn.getArgument(n).asExpr()
649+
)
489650
}
490651
}
491652

@@ -515,6 +676,13 @@ private module Mongoose {
515676
// callback provided to a Query method call
516677
exists(mcn.getCallback(mcn.getNumArgument() - 1))
517678
)
679+
or
680+
exists(string method |
681+
Document::MethodSignatures::returnsQuery(method) and
682+
mcn = Document::ref().getAMethodCall(method) and
683+
// callback provided to a Document method call
684+
exists(mcn.getCallback(mcn.getNumArgument() - 1))
685+
)
518686
)
519687
}
520688

javascript/ql/test/query-tests/Security/CWE-089/untyped/SqlInjection.expected

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,18 @@ nodes
9090
| mongoose.js:81:46:81:50 | query |
9191
| mongoose.js:82:47:82:51 | query |
9292
| mongoose.js:82:47:82:51 | query |
93+
| mongoose.js:84:46:84:50 | query |
94+
| mongoose.js:84:46:84:50 | query |
95+
| mongoose.js:86:51:86:55 | query |
96+
| mongoose.js:86:51:86:55 | query |
97+
| mongoose.js:88:46:88:50 | query |
98+
| mongoose.js:88:46:88:50 | query |
99+
| mongoose.js:91:46:91:50 | query |
100+
| mongoose.js:91:46:91:50 | query |
101+
| mongoose.js:93:51:93:55 | query |
102+
| mongoose.js:93:51:93:55 | query |
103+
| mongoose.js:95:46:95:50 | query |
104+
| mongoose.js:95:46:95:50 | query |
93105
| mongooseJsonParse.js:19:11:19:20 | query |
94106
| mongooseJsonParse.js:19:19:19:20 | {} |
95107
| mongooseJsonParse.js:20:19:20:44 | JSON.pa ... y.data) |
@@ -236,6 +248,18 @@ edges
236248
| mongoose.js:20:11:20:20 | query | mongoose.js:81:46:81:50 | query |
237249
| mongoose.js:20:11:20:20 | query | mongoose.js:82:47:82:51 | query |
238250
| mongoose.js:20:11:20:20 | query | mongoose.js:82:47:82:51 | query |
251+
| mongoose.js:20:11:20:20 | query | mongoose.js:84:46:84:50 | query |
252+
| mongoose.js:20:11:20:20 | query | mongoose.js:84:46:84:50 | query |
253+
| mongoose.js:20:11:20:20 | query | mongoose.js:86:51:86:55 | query |
254+
| mongoose.js:20:11:20:20 | query | mongoose.js:86:51:86:55 | query |
255+
| mongoose.js:20:11:20:20 | query | mongoose.js:88:46:88:50 | query |
256+
| mongoose.js:20:11:20:20 | query | mongoose.js:88:46:88:50 | query |
257+
| mongoose.js:20:11:20:20 | query | mongoose.js:91:46:91:50 | query |
258+
| mongoose.js:20:11:20:20 | query | mongoose.js:91:46:91:50 | query |
259+
| mongoose.js:20:11:20:20 | query | mongoose.js:93:51:93:55 | query |
260+
| mongoose.js:20:11:20:20 | query | mongoose.js:93:51:93:55 | query |
261+
| mongoose.js:20:11:20:20 | query | mongoose.js:95:46:95:50 | query |
262+
| mongoose.js:20:11:20:20 | query | mongoose.js:95:46:95:50 | query |
239263
| mongoose.js:20:19:20:20 | {} | mongoose.js:20:11:20:20 | query |
240264
| mongoose.js:21:19:21:26 | req.body | mongoose.js:21:19:21:32 | req.body.title |
241265
| mongoose.js:21:19:21:26 | req.body | mongoose.js:21:19:21:32 | req.body.title |
@@ -285,6 +309,18 @@ edges
285309
| mongoose.js:21:19:21:32 | req.body.title | mongoose.js:81:46:81:50 | query |
286310
| mongoose.js:21:19:21:32 | req.body.title | mongoose.js:82:47:82:51 | query |
287311
| mongoose.js:21:19:21:32 | req.body.title | mongoose.js:82:47:82:51 | query |
312+
| mongoose.js:21:19:21:32 | req.body.title | mongoose.js:84:46:84:50 | query |
313+
| mongoose.js:21:19:21:32 | req.body.title | mongoose.js:84:46:84:50 | query |
314+
| mongoose.js:21:19:21:32 | req.body.title | mongoose.js:86:51:86:55 | query |
315+
| mongoose.js:21:19:21:32 | req.body.title | mongoose.js:86:51:86:55 | query |
316+
| mongoose.js:21:19:21:32 | req.body.title | mongoose.js:88:46:88:50 | query |
317+
| mongoose.js:21:19:21:32 | req.body.title | mongoose.js:88:46:88:50 | query |
318+
| mongoose.js:21:19:21:32 | req.body.title | mongoose.js:91:46:91:50 | query |
319+
| mongoose.js:21:19:21:32 | req.body.title | mongoose.js:91:46:91:50 | query |
320+
| mongoose.js:21:19:21:32 | req.body.title | mongoose.js:93:51:93:55 | query |
321+
| mongoose.js:21:19:21:32 | req.body.title | mongoose.js:93:51:93:55 | query |
322+
| mongoose.js:21:19:21:32 | req.body.title | mongoose.js:95:46:95:50 | query |
323+
| mongoose.js:21:19:21:32 | req.body.title | mongoose.js:95:46:95:50 | query |
288324
| mongooseJsonParse.js:19:11:19:20 | query | mongooseJsonParse.js:23:19:23:23 | query |
289325
| mongooseJsonParse.js:19:11:19:20 | query | mongooseJsonParse.js:23:19:23:23 | query |
290326
| mongooseJsonParse.js:19:19:19:20 | {} | mongooseJsonParse.js:19:11:19:20 | query |
@@ -357,6 +393,12 @@ edges
357393
| mongoose.js:76:10:76:14 | query | mongoose.js:21:19:21:26 | req.body | mongoose.js:76:10:76:14 | query | This query depends on $@. | mongoose.js:21:19:21:26 | req.body | a user-provided value |
358394
| mongoose.js:81:46:81:50 | query | mongoose.js:21:19:21:26 | req.body | mongoose.js:81:46:81:50 | query | This query depends on $@. | mongoose.js:21:19:21:26 | req.body | a user-provided value |
359395
| mongoose.js:82:47:82:51 | query | mongoose.js:21:19:21:26 | req.body | mongoose.js:82:47:82:51 | query | This query depends on $@. | mongoose.js:21:19:21:26 | req.body | a user-provided value |
396+
| mongoose.js:84:46:84:50 | query | mongoose.js:21:19:21:26 | req.body | mongoose.js:84:46:84:50 | query | This query depends on $@. | mongoose.js:21:19:21:26 | req.body | a user-provided value |
397+
| mongoose.js:86:51:86:55 | query | mongoose.js:21:19:21:26 | req.body | mongoose.js:86:51:86:55 | query | This query depends on $@. | mongoose.js:21:19:21:26 | req.body | a user-provided value |
398+
| mongoose.js:88:46:88:50 | query | mongoose.js:21:19:21:26 | req.body | mongoose.js:88:46:88:50 | query | This query depends on $@. | mongoose.js:21:19:21:26 | req.body | a user-provided value |
399+
| mongoose.js:91:46:91:50 | query | mongoose.js:21:19:21:26 | req.body | mongoose.js:91:46:91:50 | query | This query depends on $@. | mongoose.js:21:19:21:26 | req.body | a user-provided value |
400+
| mongoose.js:93:51:93:55 | query | mongoose.js:21:19:21:26 | req.body | mongoose.js:93:51:93:55 | query | This query depends on $@. | mongoose.js:21:19:21:26 | req.body | a user-provided value |
401+
| mongoose.js:95:46:95:50 | query | mongoose.js:21:19:21:26 | req.body | mongoose.js:95:46:95:50 | query | This query depends on $@. | mongoose.js:21:19:21:26 | req.body | a user-provided value |
360402
| mongooseJsonParse.js:23:19:23:23 | query | mongooseJsonParse.js:20:30:20:43 | req.query.data | mongooseJsonParse.js:23:19:23:23 | query | This query depends on $@. | mongooseJsonParse.js:20:30:20:43 | req.query.data | a user-provided value |
361403
| mongooseModelClient.js:11:16:11:24 | { id: v } | mongooseModelClient.js:10:22:10:29 | req.body | mongooseModelClient.js:11:16:11:24 | { id: v } | This query depends on $@. | mongooseModelClient.js:10:22:10:29 | req.body | a user-provided value |
362404
| mongooseModelClient.js:12:16:12:34 | { id: req.body.id } | mongooseModelClient.js:12:22:12:29 | req.body | mongooseModelClient.js:12:16:12:34 | { id: req.body.id } | This query depends on $@. | mongooseModelClient.js:12:22:12:29 | req.body | a user-provided value |

0 commit comments

Comments
 (0)