JavaScript NoSQL Injection prevention in MongoDB
Asked Answered
C

5

32

How can I prevent JavaScript NoSQL injections into MongoDB?

I am working on a Node.js application and I am passing req.body, which is a json object, into the mongoose model's save function. I thought there were safeguards behind the scenes, but this doesn't appear to be the case.

Cila answered 18/11, 2012 at 1:0 Comment(6)
As long as you're parsing the JSON using JSON.parse (as opposed to eval), and you're validating the data, you should be fine. JSON isn't inherently dangerous :)Roth
Don't prevent things from entering your database, prevent things in your database from being run as code.Stanhope
So how do I parse my post object?Cila
My code looks like this : module.exports.create = (post, cb) -> post.created_at = Date.now() PostModel(post).save (err, post) -> cb(err, post)Cila
PostModel(JSON.parse(post)).save gives an error - Unexpected token oCila
+Incognito that sounds like great advice, but it's not. You need to be distrustful of any data being received, and validate & sanitize them before using or storing.Cully
F
-3

Note My answer is incorrect. Please refer to other answers.

--

As a client program assembles a query in MongoDB, it builds a BSON object, not a string. Thus traditional SQL injection attacks are not a problem.

For details follow the documentation

UPDATE

Avoid expression like eval which can execute arbitrary JS. If you are taking input from user and running eval like expressions without cleaning the input you can screw up. As pointed by JoBu1324, operations like where, mapReduce and group permit to execute JS expressions directly.

Fudge answered 18/11, 2012 at 9:2 Comment(9)
I'd add a "However, beware of JavaScript injection!" to the answer. Read media.blackhat.com/bh-us-11/Sullivan/… .Groceryman
Considering the node.js tag and javascript subsection in the docs, is this answer wrong? I'm searching for the answer to this question myself right now.Callas
@Callas I have updated the answer. Thanks for improving the answer :)Fudge
Please don't vandalize your answer.Ionia
@RobertColumbia How is it vandalizing? My answer is incorrect. If its not adding any value why not just remove it.Fudge
It was accepted by the OP, therefore it was deemed to have some value.Ionia
But the fact that my answer is misleading and the person unfortunately relied on it doesn't mean it has value. Can't get my head around how misleading information is helping anyone. @RobertColumbiaFudge
@RobertColumbia Solely because OP doesn't know what is true and what is not, that's the reason he even needed to ask the question. It might be good idea if you SushantGupta would flag his question and ask moderator to remove the accepted status. Other alternative is to clearly say that your answer is incorrect. "might no longer be relevant" is a big understatement.Rorqual
@Rorqual Agreed. My answer is incorrect. Updated.Fudge
C
48

Sushant's answer is not correct. You need to be aware of NoSQL injection in MongoDB.

Example (taken from here)

User.findOne({
    "name" : req.params.name, 
    "password" : req.params.password
}, callback); 

If req.params.password is { $ne: 1 }, the user will be retrieved without knowing the password ($ne means not equals 1).

MongoDB Driver

You can use mongo-sanitize:

It will strip out any keys that start with '$' in the input, so you can pass it to MongoDB without worrying about malicious users overwriting.

var sanitize = require('mongo-sanitize');

var name = sanitize(req.params.name);
var password = sanitize(req.params.password);

User.findOne({
    "name" : name, 
    "password" : password
}, callback); 

Mongoose Driver

As it follows a schema, if the password is a string field, it will convert the object { $ne: 1 } to string and no damage will be done. In this case, you don't need to sanitize, just remember to set a proper schema.

Comyns answered 16/7, 2016 at 21:49 Comment(9)
i just tried this with req.params.password='{ $ne: 1 }' and did not get a result, are you sure that the string req.params.password has its json contents parsed?Shulem
@MohammadAli no, the JSON will not be parsed automatically. The problem is when you receive an object { $ne: 1 } instead of a string '{ $ne: 1 }'. I've wrote about this with more details in my blog: link.Comyns
shouldent that be mentioned in the answer? as what youve written about the mongoose driver also applies to the native mongodb driver. and you also neglect to mention that the type needs to be an object for the example given to workShulem
@MohammadAli it was already mentioned in the answer. I've said If req.params.password is { $ne: 1 }. The { $ne: 1 } in JavaScript represents an object and not a string. Regarding Mongoose, I said that it will convert objects into strings. It was emphasized in the end: it will convert the object { $ne: 1 } to stringComyns
when using mongoose, is it possible to do this in the .pre('save') function ? or is it too late by then?Gautier
This is a confusing answer. In standard request parsing, req.params.password could never be an object, only a string.Forth
If we specify content-type to be application/json how it's possible to receive an object?Perfuse
The trick is to pass request like req.params.password['$ne']=1 This may let to "password" : { $ne: "1" } without sanitize.Distress
Simply put, no it's not so easy to inject mongoDB AS LONG AS YOU HANDLE YOUR TYPES, if you take a user req.params, parse it to JSON and then use the input directly in mongoDB driver yes it is sensible. However if you either make sure the supplied input is a string either by casting it using String(req.params.password) or by using mongoose schema, then you'll be fine. I'd be more worried of eval though.Polak
H
13

Although the post is obsolete, I'm answering.

I know three ways.

First: There is a multipurpose content-filter. Also provides MongoDB injection protection by filtering way.

Second: mongo-sanitize, Helper to sanitize mongodb queries against query selector injections.

Third: I'd seen over here this solution which can be applied for MongoDB too. It's really simple to implement. Only use built-in escape() function of JavaScript.

escape() converts the string into ascii code. $ne is converted into %24ne.

var privateKey = escape(req.params.privateKey);

App.findOne({ key: privateKey }, function (err, app) {
  //do something here
}
Hallowell answered 8/8, 2015 at 21:50 Comment(0)
B
4

In order to guard against query selector injections from a data object with unknown structure

Use mongo-sanitize to deeply sanitize via recursion:

const deepSanitize = (value) => {
    if(Array.isArray(value)){
        value.forEach(elm=>deepSanitize(elm))
    }
    if(typeof(value) === 'object' && value !== null){
        Object.values(value).forEach((elm)=>{
            deepSanitize(elm)
        })
    }
    return sanitize(value)
}

For example with sanitize(req.query) nested query selectors will not be removed:

const req = {} 
req.query = { _id : { $ne: 1 } } 

console.log(req.query))               // { _id: { '$ne': 1 } }
console.log(sanitize(req.query))      // { _id: { '$ne': 1 } }

Using deepSanitize(req.query) sanitized objects (including nested) are mutated:

console.log(deepSanitize(req.query))       // { _id: {} }
console.log(req.query)                     // { _id: {} }

Eliminate object mutation with {...req.query}:

console.log(deepSanitize({...req.query}))  // { _id: {} }
console.log(req.query)                     // { _id: { '$ne': 1 } }
Bartolommeo answered 22/10, 2019 at 19:33 Comment(1)
Just use a mongoose schema that will cast your inputs as string. If you're paranoid you can sanitize but since your input become's a string at request level it makes it impossible for mongo to treat it as anything else. However if you plan on inserting objects you MUST use mongo-sanitise to remove all of mongo's special keys.Polak
U
2

If you are using Mongoose in Mongoose 6 they introduced the sanitizeFilter option that could be used as follows (from the their documentation):

const obj = { username: 'val', pwd: { $ne: null } };
sanitizeFilter(obj);
obj; // { username: 'val', pwd: { $eq: { $ne: null } } });

Sanitizes query filters against query selector injection attacks by wrapping any nested objects that have a property whose name starts with $ in a $eq.

You can also set it up to be sensitized by default:

mongoose.set('sanitizeFilter', true);

And you can also skip the default sensitizing by using trusted():

const user = await User.findOne({
  // Tell Mongoose to not sanitize `{ $ne: true }`
  deleted: mongoose.trusted({ $ne: true }),
  email: req.body.email,
  hashedPassword: req.body.hashedPassword
}).setOptions({ sanitizeFilter: true }); 
Untold answered 3/1, 2023 at 11:15 Comment(0)
F
-3

Note My answer is incorrect. Please refer to other answers.

--

As a client program assembles a query in MongoDB, it builds a BSON object, not a string. Thus traditional SQL injection attacks are not a problem.

For details follow the documentation

UPDATE

Avoid expression like eval which can execute arbitrary JS. If you are taking input from user and running eval like expressions without cleaning the input you can screw up. As pointed by JoBu1324, operations like where, mapReduce and group permit to execute JS expressions directly.

Fudge answered 18/11, 2012 at 9:2 Comment(9)
I'd add a "However, beware of JavaScript injection!" to the answer. Read media.blackhat.com/bh-us-11/Sullivan/… .Groceryman
Considering the node.js tag and javascript subsection in the docs, is this answer wrong? I'm searching for the answer to this question myself right now.Callas
@Callas I have updated the answer. Thanks for improving the answer :)Fudge
Please don't vandalize your answer.Ionia
@RobertColumbia How is it vandalizing? My answer is incorrect. If its not adding any value why not just remove it.Fudge
It was accepted by the OP, therefore it was deemed to have some value.Ionia
But the fact that my answer is misleading and the person unfortunately relied on it doesn't mean it has value. Can't get my head around how misleading information is helping anyone. @RobertColumbiaFudge
@RobertColumbia Solely because OP doesn't know what is true and what is not, that's the reason he even needed to ask the question. It might be good idea if you SushantGupta would flag his question and ask moderator to remove the accepted status. Other alternative is to clearly say that your answer is incorrect. "might no longer be relevant" is a big understatement.Rorqual
@Rorqual Agreed. My answer is incorrect. Updated.Fudge

© 2022 - 2024 — McMap. All rights reserved.