$eq, $gt, $in, etc. and injection

[Sidney San Martín]

Hey,

The page in the docs which talks about injection says this:

Querying.  Generally this is not a problem as for { x : user_obj }, dollar signs are not top level and have no effect. In theory one might let the user build a query completely themself and provide it to the database. In that case checking for $ characters in keynames is important. That however would be a highly unusual case.

One way to handle user-generated keys is to always put them in sub-objects. Then they are never at top level (where $operators live) anyway.

But, that's not really the case. There are a bunch of operators — $exists, $gt, $in, etc. that don't just work at the top level. If a website has a JSON API, something like this could happen:

// A malicious user's login request:

{

"username": "admin",

"password": { "$exists": true }

}

// Somewhere deep inside the server…

var user = db.users.find({ username: request.username, password: request.password });

if (user) {

// …

}

As far as I can tell, that works (yes, storing plain text passwords in the database would be a bad idea — it's just an example).

Do any of the drivers provide ways to stop this kind of thing?

[Dwight Merriman]

the assumption in that commentation was that the parameters are strings rather than arbitrary json objects being passed through to the db.   so yes, don't pass through json from an untrusted client as json/bson.

--
You received this message because you are subscribed to the Google Groups "mongodb-dev" group.
To view this discussion on the web visit https://groups.google.com/d/msg/mongodb-dev/-/LZjKXrZBQxoJ.
To post to this group, send email to mongo...@googlegroups.com.
To unsubscribe from this group, send email to mongodb-dev...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/mongodb-dev?hl=en.

[Scott Hernandez]

Sidney, as Dwight has indicated, if you aren't evaluating the whole
string, including the user parameter then this isn't an issue. Even in
dynamic languages like python It will just pass the user contributed
string along as a single parameter and the server will not expand or
evaluate it in any way like you are implying.

I hope that makes the distinction clear between input data and queries
(the bson/object structure). If you need an example of what you have
to do to go out of your way to do to enable this kind of hole I'd be
happy to provide a simple example.

[Sidney San Martín]

Hey, sorry. I thought I turned on email notifications for this topic, but it looks like it didn't stick…

 

If you need an example of what you have to do to go out of your way to do to enable this kind of hole I'd be happy to provide a simple example.

If you're talking about a website with a JSON API, like many of them these days, I think it's pretty easy to enable this kind of hole. Here's a simple Node.js server:

var express = require('express'),

mongodb = require('mongodb'),

app = express.createServer(express.bodyParser()),

db = new mongodb.Db('someDatabase', new mongodb.Server('127.0.0.1', 27017, {})),

users;

app.post('/login', function(req, res){

res.header('Content-Type', 'text/plain; charset=utf-8');

users.findOne({ username: req.body.username, password: req.body.password }, function(err, user){

if (err) {

res.send("Error");

} else if (user) {

res.send('logged in as ' + user.username + '\n');

} else {

res.send('failed to log in\n');

}

});

});

db.open(function(err){

if (err) throw err;

db.collection('users', function(err, collection){

if (err) throw err;

users = collection;

app.listen(3000);

});

});

 Try these command lines:

curl -H "Content-Type: application/json" -d '{ "username": "foo", "password": "bar" }' http://localhost:3000/login

curl -H "Content-Type: application/json" -d '{ "username": "foo", "password": "what" }' http://localhost:3000/login

curl -H "Content-Type: application/json" -d '{ "username": "foo", "password": { "$exists": true } }' http://localhost:3000/login

 Yes, you could fix the vulnerability by changing the query to { username: req.body.username.toString(), password: req.body.password.toString() }, but that's easy as hell to forget and not particularly noticeable, and I would bet real money on this kind of care not being common.

I don't know as much about using MongoDB in other languages; how would e.g. Python be safer?

[Sean Corfield]

On Sun, Dec 11, 2011 at 8:36 PM, Sidney San Martín <s...@sidneysm.com> wrote:
>  Yes, you could fix the vulnerability by changing the query to { username:
> req.body.username.toString(), password: req.body.password.toString() }, but
> that's easy as hell to forget and not particularly noticeable, and I would
> bet real money on this kind of care not being common.

I'm sorry but this sort of basic web security should be part of any
web application. Passing untreated user input directly to your
database is foolish at best and most people would consider it
unprofessional. Blaming the database for doing "bad things" when you
pass it bad data is... disingenuous...
--
Sean A Corfield -- (904) 302-SEAN
An Architect's View -- http://corfield.org/
World Singles, LLC. -- http://worldsingles.com/

"Perfection is the enemy of the good."
-- Gustave Flaubert, French realist novelist (1821-1880)

[Sidney San Martín]

On Dec 12, 2011, at 1:23 AM, Sean Corfield wrote:

> I'm sorry but this sort of basic web security should be part of any
> web application. Passing untreated user input directly to your
> database is foolish at best and most people would consider it
> unprofessional. Blaming the database for doing "bad things" when you
> pass it bad data is... disingenuous...

Why should developers need to sanitize data that they hand off to a driver?

I don’t believe that it is immediately obvious, either, how to handle user input safely, especially when it can be legitimately be stuff other than strings (for example, any API that takes its requests as JSON).

You can’t look at a line of code like

db.posts.find({ owner: user, tags: { $all: tags } });

and tell if its inputs are safe. Is `user` or `tags` untrusted? Have they both been cast to strings? If `tags` isn’t supposed to be a string, has it been filtered for operators? Can operators even be used inside $all (if not, `tags` is OK no matter what, right?) If the rules aren’t simple, it’s tricky to audit your code.

I think it would make more sense if you could tell by looking at that one line of code whether the input’s being handled in a safe way:

// May or may not be safe:
db.sessions.find({ _id: session_id, username: username });

As policy, you could decide that any value which gets used in a query must be cast to a string in the same place it’s incorporated into the query object:

// Definitely safe, if everything can a string:
db.sessions.find({ _id: session_id.toString(), username: username.toString() });

But, that doesn’t work if not all of the values are strings (`session_id` might be an Object ID, for instance). Personally, I think that it would be much nicer to let you show the driver what should be treated as data:

// Definitely safe, needs driver support
db.sessions.find({ _id: { $: session_id }, username: { $: username } });

The driver would decide how to pass it safely to the database. The responsibility would be out of developers’ hands beyond “use your driver like this”.

(There might be a better solution — but “don't pass through [untrusted] json” being the state of things makes me really uncomfortable. I’m going to open an issue on node-mongodb-native (just because that’s the driver I use) and, if I have a chance, start building support for something like this.)

[Aaron Heckmann]

Mongoose (mongoosejs.com) for nodejs uses schemas for all collections. Each query is casted to its defined type before passing it down to the driver (node-mongodb-native).

--
You received this message because you are subscribed to the Google Groups "mongodb-dev" group.
To post to this group, send email to mongo...@googlegroups.com.
To unsubscribe from this group, send email to mongodb-dev...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/mongodb-dev?hl=en.

--
Aaron

[Sidney San Martín]

On Jan 13, 2012, at 7:38 AM, Aaron Heckmann wrote:

> Mongoose (mongoosejs.com) for nodejs uses schemas for all collections. Each query is casted to its defined type before passing it down to the driver (node-mongodb-native).

Interesting… how do you tell it the difference between an intentional part of the query (e.g. `{ someValue: { $gt: value } }`) and an unintentional one (e.g. `{ someValue: valueFromUser`)?

[Scott Hernandez]

Maybe the basic misunderstanding.her is that the drivers do not send queries as strings to the server but instead send bson. The server does not evaluate strings as structured elements, with a few exceptions.

[Sidney San Martín]

On Jan 14, 2012, at 8:54 PM, Scott Hernandez wrote:
> Maybe the basic misunderstanding.her is that the drivers do not send queries as strings to the server but instead send bson. The server does not evaluate strings as structured elements, with a few exceptions.

Absolutely, and that’s something that I love about MongoDB.

Is it reasonable for me to argue that untrusted inputs aren’t necessarily strings either?

[Dwight Merriman]

> Is it reasonable for me to argue that untrusted inputs aren’t necessarily strings either?

yes.

[Aaron Heckmann]

On Sat, Jan 14, 2012 at 8:43 PM, Sidney San Martín <s...@sidneysm.com> wrote:

Interesting… how do you tell it the difference between an intentional part of the query (e.g. `{ someValue: { $gt: value } }`) and an unintentional one (e.g. `{ someValue: valueFromUser`)?

Ah i see. Input validation (object/string/etc) happens in the application before passing it to mongoose. Interested in seeing what you come up with.

--
Aaron