hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2025-12-17 01:03:14 +01:00
Code Issues Packages Projects Releases Wiki Activity

JS: add the NoSQL $where as a sink for js/code-injection

Browse Source
This commit is contained in:
Esben Sparre Andreasen
2020-05-12 22:05:30 +02:00
parent 20cf04442c
commit 7722d77c86
7 changed files with 94 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
change-notes/1.25/analysis-javascript.md
View File

@@ -25,6 +25,7 @@
| Uncontrolled command line (`js/command-line-injection`) | More results | This query now recognizes additional command execution calls. |
| Expression has no effect (`js/useless-expression`) | Less results | This query no longer flags an expression when that expression is the only content of the containing file. |
| Unknown directive (`js/unknown-directive`) | Less results | This query no longer flags directives generated by the Babel compiler. |
| Code injection (`js/code-injection`) | More results | More potential vulnerabilities involving NoSQL code operators are now recognized. |
## Changes to libraries

28
javascript/ql/src/semmle/javascript/frameworks/NoSQL.qll
View File

@@ -6,7 +6,17 @@ import javascript
module NoSQL {
/** An expression that is interpreted as a NoSQL query. */
abstract class Query extends Expr { }
abstract class Query extends Expr {
/** Gets an expression that is interpreted as a code operator in this query. */
DataFlow::Node getACodeOperator() { none() }
}
}
/**
* Gets the value of a `$where` property of an object that flows to `n`.
*/
private DataFlow::Node getADollarWherePropertyValue(DataFlow::Node n) {
result = n.getALocalSource().getAPropertyWrite("$where").getRhs()
}
/**
@@ -190,6 +200,10 @@ private module MongoDB {
*/
class Query extends NoSQL::Query {
Query() { this = any(QueryCall qc).getAQueryArgument().asExpr() }
override DataFlow::Node getACodeOperator() {
result = getADollarWherePropertyValue(this.flow())
}
}
}
@@ -678,6 +692,10 @@ private module Mongoose {
*/
class MongoDBQueryPart extends NoSQL::Query {
MongoDBQueryPart() { any(InvokeNode call).interpretsArgumentAsQuery(this.flow()) }
override DataFlow::Node getACodeOperator() {
result = getADollarWherePropertyValue(this.flow())
}
}
/**
@@ -763,6 +781,10 @@ private module Minimongo {
*/
class Query extends NoSQL::Query {
Query() { this = any(QueryCall qc).getAQueryArgument().asExpr() }
override DataFlow::Node getACodeOperator() {
result = getADollarWherePropertyValue(this.flow())
}
}
}
@@ -809,5 +831,9 @@ private module MarsDB {
*/
class Query extends NoSQL::Query {
Query() { this = any(QueryCall qc).getAQueryArgument().asExpr() }
override DataFlow::Node getACodeOperator() {
result = getADollarWherePropertyValue(this.flow())
}
}
}

2
javascript/ql/src/semmle/javascript/heuristics/AdditionalSinks.qll
View File

@@ -25,6 +25,8 @@ abstract class HeuristicSink extends DataFlow::Node { }
private class HeuristicCodeInjectionSink extends HeuristicSink, CodeInjection::Sink {
HeuristicCodeInjectionSink() {
isAssignedTo(this, "$where")
or
isAssignedToOrConcatenatedWith(this, "(?i)(command|cmd|exec|code|script|program)")
or
isArgTo(this, "(?i)(eval|run)") // "exec" clashes too often with `RegExp.prototype.exec`

7
javascript/ql/src/semmle/javascript/security/dataflow/CodeInjectionCustomizations.qll
View File

@@ -115,4 +115,11 @@ module CodeInjection {
)
}
}
/**
* A code operator of a NoSQL query as a code injection sink.
*/
class NoSQLCodeInjectionSink extends Sink {
NoSQLCodeInjectionSink() { any(NoSQL::Query q).getACodeOperator() = this }
}
}

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

@@ -1,4 +1,13 @@
nodes
| NoSQLCodeInjection.js:18:24:18:31 | req.body |
| NoSQLCodeInjection.js:18:24:18:31 | req.body |
| NoSQLCodeInjection.js:18:24:18:37 | req.body.query |
| NoSQLCodeInjection.js:18:24:18:37 | req.body.query |
| NoSQLCodeInjection.js:19:24:19:48 | "name = ... dy.name |
| NoSQLCodeInjection.js:19:24:19:48 | "name = ... dy.name |
| NoSQLCodeInjection.js:19:36:19:43 | req.body |
| NoSQLCodeInjection.js:19:36:19:43 | req.body |
| NoSQLCodeInjection.js:19:36:19:48 | req.body.name |
| angularjs.js:10:22:10:29 | location |
| angularjs.js:10:22:10:29 | location |
| angularjs.js:10:22:10:36 | location.search |
@@ -108,6 +117,14 @@ nodes
| tst.js:26:26:26:53 | locatio ... ring(1) |
| tst.js:26:26:26:53 | locatio ... ring(1) |
edges
| NoSQLCodeInjection.js:18:24:18:31 | req.body | 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 |
| NoSQLCodeInjection.js:18:24:18:31 | req.body | 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 |
| NoSQLCodeInjection.js:19:36:19:43 | req.body | NoSQLCodeInjection.js:19:36:19:48 | req.body.name |
| NoSQLCodeInjection.js:19:36:19:43 | req.body | NoSQLCodeInjection.js:19:36:19:48 | req.body.name |
| NoSQLCodeInjection.js:19:36:19:48 | req.body.name | NoSQLCodeInjection.js:19:24:19:48 | "name = ... dy.name |
| NoSQLCodeInjection.js:19:36:19:48 | req.body.name | NoSQLCodeInjection.js:19:24:19:48 | "name = ... dy.name |
| angularjs.js:10:22:10:29 | location | angularjs.js:10:22:10:36 | location.search |
| angularjs.js:10:22:10:29 | location | angularjs.js:10:22:10:36 | location.search |
| angularjs.js:10:22:10:29 | location | angularjs.js:10:22:10:36 | location.search |
@@ -212,6 +229,8 @@ edges
| tst.js:26:26:26:40 | location.search | tst.js:26:26:26:53 | locatio ... ring(1) |
| tst.js:26:26:26:40 | location.search | tst.js:26:26:26:53 | locatio ... ring(1) |
#select
| 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 |
| 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 |
| 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 |
| 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 |
| 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 |

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

@@ -1,4 +1,13 @@
nodes
| NoSQLCodeInjection.js:18:24:18:31 | req.body |
| NoSQLCodeInjection.js:18:24:18:31 | req.body |
| NoSQLCodeInjection.js:18:24:18:37 | req.body.query |
| NoSQLCodeInjection.js:18:24:18:37 | req.body.query |
| NoSQLCodeInjection.js:19:24:19:48 | "name = ... dy.name |
| NoSQLCodeInjection.js:19:24:19:48 | "name = ... dy.name |
| NoSQLCodeInjection.js:19:36:19:43 | req.body |
| NoSQLCodeInjection.js:19:36:19:43 | req.body |
| NoSQLCodeInjection.js:19:36:19:48 | req.body.name |
| angularjs.js:10:22:10:29 | location |
| angularjs.js:10:22:10:29 | location |
| angularjs.js:10:22:10:36 | location.search |
@@ -112,6 +121,14 @@ nodes
| tst.js:26:26:26:53 | locatio ... ring(1) |
| tst.js:26:26:26:53 | locatio ... ring(1) |
edges
| NoSQLCodeInjection.js:18:24:18:31 | req.body | 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 |
| NoSQLCodeInjection.js:18:24:18:31 | req.body | 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 |
| NoSQLCodeInjection.js:19:36:19:43 | req.body | NoSQLCodeInjection.js:19:36:19:48 | req.body.name |
| NoSQLCodeInjection.js:19:36:19:43 | req.body | NoSQLCodeInjection.js:19:36:19:48 | req.body.name |
| NoSQLCodeInjection.js:19:36:19:48 | req.body.name | NoSQLCodeInjection.js:19:24:19:48 | "name = ... dy.name |
| NoSQLCodeInjection.js:19:36:19:48 | req.body.name | NoSQLCodeInjection.js:19:24:19:48 | "name = ... dy.name |
| angularjs.js:10:22:10:29 | location | angularjs.js:10:22:10:36 | location.search |
| angularjs.js:10:22:10:29 | location | angularjs.js:10:22:10:36 | location.search |
| angularjs.js:10:22:10:29 | location | angularjs.js:10:22:10:36 | location.search |

21
javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/NoSQLCodeInjection.js Normal file
View File

@@ -0,0 +1,21 @@
const express = require("express"),
mongodb = require("mongodb"),
bodyParser = require("body-parser");
const MongoClient = mongodb.MongoClient;
const app = express();
app.use(bodyParser.urlencoded({ extended: true }));
app.post("/documents/find", (req, res) => {
const query = {};
query.title = req.body.title;
MongoClient.connect("mongodb://localhost:27017/test", (err, db) => {
let doc = db.collection("doc");
doc.find(query); // NOT OK, but that is flagged by js/sql-injection
doc.find({ $where: req.body.query }); // NOT OK
doc.find({ $where: "name = " + req.body.name }); // NOT OK
});
});