Mongoose: Nosql injections

[mfrachet]

Does mongoose protect against NoSql injections or should I implement it myself ?

[vkarpov15]

Which injections? Mongoose casting doesn't necessarily protect you, I'd recommend using $eq for user provided data

[fritx]

FYI, I'm not sure whether mongoose is doing some protection but I used to handle it by myself while receiving a request in koa, using https://github.com/vkarpov15/mongo-sanitize/blob/master/index.js like

// for query
apiRouter.use(async (ctx, next) => {
  sanitize(ctx.query)
  await next()
})

// for json body
koaJson = koaCompose([
  koaBody,
  koaContentType('application/json'),
  async (ctx, next) => {
    let { body } = ctx.request
    sanitize(body)
    ctx.request.body = body next()
  },
])

let mongoSanitize = require('mongo-sanitize')

function sanitize (obj) {
  mongoSanitizeRecurse(obj)
}
function mongoSanitizeRecurse (obj) {
  mongoSanitize(obj)
  _.each(obj, v => {
    if (_.isObject(v)) {
      mongoSanitizeRecurse(v)
    }
  })
}

[Fusseldieb]

Does mongoose protect against NoSql injections or should I implement it myself ?

Just tested it out. Using Mongoose and Express, I could "hack" myself into my own server by using:

{
    "username": {"$ne": null},
    "password": {"$ne": null}
}

The Express part is:

const user = await accountsModel.findOne({ username: req.body.username });

No, it doesn't protect you!

Use mongo-sanitize or any equivalent solution to strip out the "$" character. This character can do some nasty stuff.

If you need the "$" character, you could check if the req.body.variable is typeof === 'string'. If someone sends a payload containing a JSON (which from what I've seen is _always_ the case, it'll be rejected).

[vkarpov15]

@Fusseldieb that's a good point, Mongoose should have a way to make sanitization happen automatically for cases like this.

[Fusseldieb]

@Fusseldieb that's a good point, Mongoose should have a way to make sanitization happen automatically for cases like this.

Thanks for answering so fast. Would be awesome seeing some improvement on this part!

I don't know if it's easy or even possible, because JSON is passed into the findOne Object (or any other) and _becomes_ part of that object, making MongoDB/Mongoose parse it as a command. I wouldn't know a way to separate that in the query part. Maybe in Express' app.use() creating a hook which santizes the inputs or something...

Anyways, thanks!

[vkarpov15]

I think the easiest approach is to add a mongoose.sanitize() function that strips out dangerous keys, but that wouldn't quite be automatic.

Another idea would be to add a flag that sanitizes queries unless you wrap the query field in a new function mongoose.safe():

const schema = Schema({
  password: String
}, { sanitize: true });

// Later

await Model.findOne({ password: { $ne: null } }); // Throws an error

await Model.findOne({ password: mongoose.safe({ $ne: null }) }); // Works

What do you think @Fusseldieb ?

[e-compton]

Having a mongoose.sanitize() function would be really useful for me. This kind of use case seems to pop up a lot in our codebase:

Model.findOne({
  a: { $le: new Date() },
  b: mongoose.sanitize(potentiallyDangerousUserJSON)
});

There's already some npm packages out there doing mongo operator sanitization, but I would much prefer this functionality was built into mongoose. So that I don't have to worry about it being updated.
https://www.npmjs.com/package/mongo-sanitize

[AbdelrahmanHafez]

How can a user take advantage of { $ne: null } ?

I came to a practice that may be related to this issue:

const orders = await Order.find({ ...req.query, userId: req.user._id });

This way, we make sure that the user only has access to their own documents, and can not access other user's documents in any way. We could also add more rules to it, for example, we could add:

const orders = await Order.find({ ...req.query, userId: req.user._id, isDeleted: false, isApproved: true });

As a backend developer, I give the API consumer (mobile/front-end) a single endpoint for listing documents, and count on them to pass a query via req.query (if they need to filter for a specific page), and inject my own filters into that query as above.

Am I missing something?

[vkarpov15]

Express uses an 'extended' query parser by default, which means req.query can contain nested objects unless you explicitly disable the extended parser. So unless you're careful about overwriting potentially sensitive fields, you can get bit by an injection. I agree that the way @AbdelrahmanHafez is doing this is great, but I'd like to at least have something explicit in place to de-risk.

[Fusseldieb]

I think the easiest approach is to add a mongoose.sanitize() function that strips out dangerous keys, but that wouldn't quite be automatic.

What do you think @Fusseldieb ?

I think it would be a nice addition and an easy to use one.