diff --git a/python/ql/lib/semmle/python/frameworks/NoSQL.qll b/python/ql/lib/semmle/python/frameworks/NoSQL.qll index ba17d593a60..d7a0848ae9c 100644 --- a/python/ql/lib/semmle/python/frameworks/NoSQL.qll +++ b/python/ql/lib/semmle/python/frameworks/NoSQL.qll @@ -99,9 +99,6 @@ private module NoSql { ] } - /** Gets the name of a mongo query operator that will interpret JavaScript. */ - private string mongoQueryOperator() { result in ["$where", "$function"] } - /** * Gets a reference to a `Mongo` collection method call * @@ -125,12 +122,34 @@ private module NoSql { override predicate vulnerableToStrings() { none() } } - private class MongoCollectionQueryOperator extends API::CallNode, NoSqlQuery::Range { + /** The `$where` query operator executes a string as JavaScript. */ + private class WhereQueryOperator extends API::CallNode, NoSqlQuery::Range { DataFlow::Node query; - MongoCollectionQueryOperator() { + WhereQueryOperator() { this = mongoCollection().getMember(mongoCollectionMethodName()).getACall() and - query = this.getParameter(0).getSubscript(mongoQueryOperator()).asSink() + query = this.getParameter(0).getSubscript("$where").asSink() + } + + override DataFlow::Node getQuery() { result = query } + + override predicate interpretsDict() { none() } + + override predicate vulnerableToStrings() { any() } + } + + /** The `$function` query operator executes its `body` string as JavaScript. */ + private class FunctionQueryOperator extends API::CallNode, NoSqlQuery::Range { + DataFlow::Node query; + + FunctionQueryOperator() { + this = mongoCollection().getMember(mongoCollectionMethodName()).getACall() and + query = + this.getParameter(0) + .getASubscript*() + .getSubscript("$function") + .getSubscript("body") + .asSink() } override DataFlow::Node getQuery() { result = query } diff --git a/python/ql/test/query-tests/Security/CWE-943-NoSqlInjection/PoC/server.py b/python/ql/test/query-tests/Security/CWE-943-NoSqlInjection/PoC/server.py index a94583d8177..6a0099a4f5d 100644 --- a/python/ql/test/query-tests/Security/CWE-943-NoSqlInjection/PoC/server.py +++ b/python/ql/test/query-tests/Security/CWE-943-NoSqlInjection/PoC/server.py @@ -46,30 +46,29 @@ def by_where(): post = posts.find_one({'$where': 'this.author === "'+author+'"'}) # $ result=BAD return show_post(post, author) - @app.route('/byFunction', methods=['GET']) def by_function(): author = request.args['author'] search = { - "body": 'function(author) { return(author === "'+author+'") }', + "body": 'function(author) { return(author === "'+author+'") }', # $ result=BAD "args": [ "$author" ], "lang": "js" } # Use `" | "a" === "a` as author # making the query `this.author === "" | "a" === "a"` # Found by http://127.0.0.1:5000/byFunction?author=%22%20|%20%22a%22%20===%20%22a - post = posts.find_one({'$expr': {'$function': search}}) # $ MISING: result=BAD + post = posts.find_one({'$expr': {'$function': search}}) # $ result=BAD return show_post(post, author) @app.route('/byFunctionArg', methods=['GET']) def by_function_arg(): author = request.args['author'] search = { - "body": 'function(author, target) { return(author === target) }', + "body": 'function(author, target) { return(author === target) }', # $ result=OK "args": [ "$author", author ], "lang": "js" } - post = posts.find_one({'$expr': {'$function': search}}) # $ result=OK + post = posts.find_one({'$expr': {'$function': search}}) # $ SPURIOUS: result=BAD return show_post(post, author) @app.route('/', methods=['GET'])