CODEWITHSUNDEEP
node.jsmongodbmongoose
citivin
citivin

Reputation: 686

Preventing NoSQL injection: Isn't mongoose supposed to convert inputs based on given schema?

Looking to prevent NoSQL injection attacks for a node.js app using mongodb. 

var mongoose = require('mongoose'); // "^5.5.9"
var Schema = mongoose.Schema;

var historySchema = new Schema({
  userId: {
    type: String,
    index: true,
  },
  message: {},
  date: {
    type: Date,
    default: Date.now,
  }
});

var history = mongoose.model('history', historySchema);

// the following is to illustrate the logic, not actual code
function getHistory(user){

  history.find({userId: user}, function(err, docs) {
    console.log(docs)
  }

}

Based on this answer to a similar question, my understanding is that using mongoose and defining the field as string should prevent query injection. However, by changing the user input to a query object, it is possible to return all users. For example:

getHistory({$ne: 1}) // returns the history for all users

I am aware of other ways to prevent this type of attack before it gets to the mongoose query, like using mongo-sanitize. But I'd like to know if there's something wrong with the way I defined the schema or if one can't expect mongoose to convert inputs according to the schema.

Thanks in advance!

Upvotes: 3

Views: 4147

Answers (2)

Len Joseph
Len Joseph

Reputation: 1458

Following the "skinny controllers, fat model" paradigm, it would be best to expose a custom validation schema from your model to be used in your controller for POST and PUT requests. This means that any data that attempts to enter your database will first be sanitized against a validation schema. Every Mongoose model should own its own validation schema.

My personal favorite for this is Joi. It's relatively simple and effective. Here is a link to the documentation: https://www.npmjs.com/package/@hapi/joi

A Joi schema permits type checking (i.e., Boolean vs. String vs. Number, etc), mandatory inputs if your document has the field required, and other type-specific enforcement such as "max" for numbers, enumerable values, etc.

Here is an example you'd include in your model:

const Joi = require('joi');
...
function validateHistory(history) {
  const historySchema = {
    userId: Joi.string(),
    message: Joi.object(),
    date: Joi.date()
  }
   return Joi.validate(history, historySchema);
}
...
module.exports.validate = validateHistory;

And then in your controller you can do:

const {
validate
} = require('../models/history');
...
router.post('/history', async (req, res) => {
    const {
      error
    } = validate(req.body.data);
    if (error) return res.status(400).send(error.details[0].message);



  let history = new History({
      userID: req.body.user,
      message: req.body.message,
      date: req.body.date
    })

    history = await history.save();
    res.send(history);
});

*Note that in a real app this route would also have an authentication callback before handling the request.

Upvotes: 0

libik
libik

Reputation: 23029

this part is good enough, you do not need anything else there. There is method that receives string and uses the string.

The best approach is to validate the input that can be modified (usually HTTP request) on top level before processing anything (I can recommend https://github.com/hapijs/joi its easy to use and you can check if there all required fields and if all fields are in correct format).

So put the validation into middleware just before it hits your controller. Or at the beginning of your controller.

From that point you are in full control of all the code and you believe what you got through your validation, so it cannot happen that someone pass object instead of string and get through.

Upvotes: 0

Related Questions