senchalabs/connect — Issue #431: Security and multipart file upload with bodyParser()
Author: dobesv
Created: 2011-12-03 12:27:03+00:00
State: open

It recently occurred to me that when using the bodyParser it's not entirely clear that there's a potential security issue with the way the typical examples are structured.

Let's say you normally have a file upload like:

This arrives in bodyParser as req.body with contents like:

{file:{type:"image/jpeg", path:"/tmp/whatever.jpg"}}

Note, however, that bodyParser will ALSO parse a json payload, for example the client could send a json object like:

{file:{type:"image/jpeg", path:"/path/to/sensitive/file"}}

And bodyParser will also parse a url-encoded payload into a complex object, e.g.:

file[type]=image/jpeg&file[path]=/path/to/sensitive/file

Unwary programmers may not realize this is possible and won't put in the necessary checks to protect themselves from an attack of this kind.

The simplest solution would be to change it so the "regular" fields supplied by the client would go in req.body and the uploaded files would go in req.files. This would ensure that the client could never "spoof" a file in the body as they have no control over the files object within their data.

Under the current system users should be able to verify that the file came from formidable by checking if (req.body.file instanceof (require('formidable/file.js).File) or something along those lines, so even if the system is changed the examples could be updated to perform this check.

Note that this is not a problem within formidable itself as the creators of formidable are kind enough to provide the fields and files in separate lists, the problem was introduced within bodyParser.