server accepting JSON in POST - how to secure/validate?

2,118 views
Skip to first unread message

HG

unread,
Jan 17, 2012, 3:10:10 AM1/17/12
to nod...@googlegroups.com
Hi!

I am building a proof of concept server to take in data through REST
API and storing it to MongoDB. I'm thinking of using express.js for
building the API and mongoskin for storing the data to MongoDB. I hope
to be able to build this so well that it could be taken to production
some day . i.e. I would not need to rewrite it with java...

So, the question is, how should I handle the JSON inside the app to
not have any security problems from that? Also, not later on, as I'm
storing it to MongoDB, but most likely not directly.

I'm thinkin of doing simply something like this:
var express = require('express');
var app = express.createServer();
app.use(express.bodyParser());

app.post('/insert/', function (req, res) {
if (!req.is('json')) {
console.log('Content-Type is not application/json');
res.writeHead(400, {'Content-Type': 'text/plain'});
res.end('Error: Invalid Content-Type\n');
} else {
var data = req.body;
// store data to MongoDB
res.writeHead(200, {'Content-Type': 'text/plain'});
res.end();
}
});

All I know is that I'm going to try to pick out fields from the data
and assemble a new object out from those that I will then store to
mongo. I.e. something like this:

mongo.save({'field1': data.field1, 'field2': data.field2})

But is that safe? And if not, how should I validate the JSON that I received?

--
HG.

Shinuza

unread,
Jan 17, 2012, 3:24:56 AM1/17/12
to nod...@googlegroups.com
Hey,

As long as you don't evaluate (eval) the data I don't think this should be a security problem. bodyParser relies on JSON.parse which will reject anything that is not valid json.

Doug Reeder

unread,
Jan 17, 2012, 8:51:10 AM1/17/12
to nod...@googlegroups.com
You might find it useful to use JSON schema to define what's allowed and what's not (and what's required). CommonJS-Utils implements a JSON validator, I believe. http://davidwalsh.name/json-validation

> --
> Job Board: http://jobs.nodejs.org/
> Posting guidelines: https://github.com/joyent/node/wiki/Mailing-List-Posting-Guidelines
> You received this message because you are subscribed to the Google
> Groups "nodejs" group.
> To post to this group, send email to nod...@googlegroups.com
> To unsubscribe from this group, send email to
> nodejs+un...@googlegroups.com
> For more options, visit this group at
> http://groups.google.com/group/nodejs?hl=en?hl=en

Doug Reeder
reed...@gmail.com
http://reeder29.livejournal.com/
https://twitter.com/reeder29

https://twitter.com/hominidsoftware
http://outlinetracker.com


Bradley Meck

unread,
Jan 17, 2012, 11:18:50 AM1/17/12
to nod...@googlegroups.com
Beware when you do not use a validator. Some code may expect properties on the schema which are not there or of a different type:

{x:1}
{x:null}
{x:"1"}

All pass hasOwnProperty and enumeration but have distinctly different effects when type coercion or property access is used. This becomes especially critical if you are checking for existence with === and the field is coerced into a string when attached to a cache.

Bradley Meck

unread,
Jan 17, 2012, 11:19:29 AM1/17/12
to nod...@googlegroups.com

HG

unread,
Jan 19, 2012, 2:48:43 PM1/19/12
to nod...@googlegroups.com
Hi!

Thanks for the suggestions: CommonJS-Utils and revalidator. However,
if bodyParser relies on JSON.parse, which is this:
http://www.json.org/js.html, then I think already at that point I have
gotten rid of potential scripts, right? So, running another validator
on top of that just slows things down, but doesn't give any more
security against scripts?

Just to make sure, so I want to be sure that I don't have anybody
sending me something like (DropTables.FromMongoDB).toJavaScript or
(DropTables.FromMySQL).toPython... you get the idea, so that some day
somebody (by mistake) executes some serverside javascript in MongoDB
and happens to evaluate that.

In addition to security, I will need to validate the input so that it
has all the fields needed and is in right format and so on. Those I
was going to do "manually" but maybe those validators are exactly the
thing.


--
HG.

Bradley Meck

unread,
Jan 19, 2012, 3:13:45 PM1/19/12
to nod...@googlegroups.com
JSON.parse will remove functions, but not save you from things such as : "{hasOwnProperty:null,toString:0,valueOf:"_"}"

JSON.parse(str).hasOwnProperty(...) toString(...) etc. will throw an error

Also a real nightmare starts to show up when you have "{"key":1}" and "{"key":"1"}" and you naively use an object as a hash.

var cache = {}
function update(str) {
  var updatedObj = JSON.parse(str)
  var objs = [{key:...}] // gotten from some API call
  var isNewObject = objs.every(function(existingObj) {
    existingObj.key !== updatedObj.key
  })
  if (isNewObject) {
    cache[key] = updatedObj
  }
  else {
     cache[key] = mergeAndUpdate(cache[key], updatedObject)
  }
}

In this case "1" !== 1 and so the updated object when attached to the cache will completely replace instead of updating and merging with the existing object.

HG

unread,
Jan 20, 2012, 3:16:37 PM1/20/12
to nod...@googlegroups.com
Hi!

On Thu, Jan 19, 2012 at 10:13 PM, Bradley Meck <bradle...@gmail.com> wrote:
> JSON.parse will remove functions, but not save you from things such as :
> "{hasOwnProperty:null,toString:0,valueOf:"_"}"
>
> JSON.parse(str).hasOwnProperty(...) toString(...) etc. will throw an error

Auts, that's evil ;-) Took a while to google and learn that you can do
this: ({}).hasOwnProperty.call(str,'hasOwnProperty'). Interesting...

Apparently bodyParser relies on JSON.parse, but it is true that the
above JSON does cause problems for JSON.parse. I'm reluctant to add
large layer of bloat to handle this as this is blocking code. Might be
that the libraries are fast, but bloat anyways. So, I'm currently
thinking that this would be safe way to take in (array of) JSONs:

var express = require('express');
var app = express.createServer()
app.use(express.bodyParser());

app.post('/track/play/', function (req, res) {
var maxLengthOfArray = 50;
if (!req.is('json')) {
res.writeHead(400, {'Content-Type': 'text/plain'});
res.end('Error: 6, Invalid Content-Type\n');
} else {
var fields=[['field1', 'string'], ['field2', 'number']];
var data = req.body;
if (!Array.isArray(req.body)) {
data = [].concat(data);
}
var fields_OK = true;
for (var i = 0, len = Math.min(data.length, maxLengthOfArray);
i < len; i++){
for (var j = 0; j < fields.length; j++){
fields_OK = fields_OK && typeof(data[i][fields[j][0]])
=== fields[j][1];
}
console.log('object: ' + JSON.stringify(data[i]));
console.log('object\'s fields_OK = ' + fields_OK);
if (fields_OK){
var tmp = {};
for (var j = 0; j < fields.length; j++){
tmp[fields[j][0]] = data[i][fields[j][0]];
}
console.log('save to db: ' + JSON.stringify(tmp));


}
}
res.writeHead(200, {'Content-Type': 'text/plain'});
res.end();
}
});

What do you think?

--
HG.

Jann Horn

unread,
Jan 20, 2012, 3:30:56 PM1/20/12
to nod...@googlegroups.com
2012/1/20 HG <hg....@gmail.com>:

> On Thu, Jan 19, 2012 at 10:13 PM, Bradley Meck <bradle...@gmail.com> wrote:
>> JSON.parse(str).hasOwnProperty(...) toString(...) etc. will throw an error
>
> Auts, that's evil ;-) Took a while to google and learn that you can do
> this: ({}).hasOwnProperty.call(str,'hasOwnProperty'). Interesting...

IMO even better:
var has = Function.prototype.call.bind(Object.prototype.hasOwnProperty)

Then later:
has(obj, prop)

Dobes

unread,
Jan 21, 2012, 7:27:29 AM1/21/12
to nodejs
On Jan 20, 4:13 am, Bradley Meck <bradley.m...@gmail.com> wrote:
> JSON.parse will remove functions, but not save you from things such as :
> "{hasOwnProperty:null,toString:0,valueOf:"_"}"
>
> JSON.parse(str).hasOwnProperty(...) toString(...) etc. will throw an error

Other than causing internal server errors, though, is this a security
problem or just a nuisance?

Are there any libraries to help parse JSON "safely" that
connect.bodyParser() could be using? Is it a matter of doing
something like:

var obj = JSON.parse("{hasOwnProperty:null,toString:0,valueOf:"_"}");
Object.keys(Object.prototype).forEach(function(naughtyKey) { delete
obj[naughtyKey]; } // Probably want to cache the list of unwanted keys
on startup

Bradley Meck

unread,
Jan 21, 2012, 5:21:13 PM1/21/12
to nod...@googlegroups.com
That is a nuisance. The cache comparison and type coercion is the real security problem.

jason.桂林

unread,
Feb 29, 2012, 5:18:27 AM2/29/12
to nod...@googlegroups.com
I am the author of mongoskin, and I wrote a validation module https://github.com/guileen/node-iform to do the validation.

node-iform will verify the data in req.body, and what you want is validate JSON, you can easily adapte json it to req.body

2012/1/22 Bradley Meck <bradle...@gmail.com>
That is a nuisance. The cache comparison and type coercion is the real security problem.

--
Job Board: http://jobs.nodejs.org/
Posting guidelines: https://github.com/joyent/node/wiki/Mailing-List-Posting-Guidelines
You received this message because you are subscribed to the Google
Groups "nodejs" group.
To post to this group, send email to nod...@googlegroups.com
To unsubscribe from this group, send email to
nodejs+un...@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/nodejs?hl=en?hl=en



--
Best regards,

Jason Green
桂林


桂林

unread,
Oct 19, 2012, 7:58:50 PM10/19/12
to nod...@googlegroups.com
I use node-validate

桂林

在 2012-10-19,上午6:05,L3monPi3 <federi...@gmail.com> 写道:

Hi HG, I have a similar project in what I'm working.
Can you tell me how do you finish or how you did the validations ?

Thanks
Reply all
Reply to author
Forward
0 new messages