5

I'm working on handling file uploads using express.js and node, and have the basic functionality working. What I need is to implement some security measures -- namely, to limit uploads to certain formats (PNG, JPEG). Is there an easy way to only allow certain formats? Would it go in the body-parser?

app.use(express.bodyParser({
    uploadDir: __dirname + '/public/uploads',
    keepExtensions: true   }));

app.use(express.limit('4mb'));

Are there any other security measures that I should take into account? Is it generally a good idea to wipe EXIF data from the image?

Thanks,

Ben

bento
  • 4,846
  • 8
  • 41
  • 59

2 Answers2

6

According to the documentation for connect's bodyParser, any options are also passed to formidable, which does the actual form parsing.

According to formidable docs, you can pass your own onPart handler:

incomingForm.onPart(part)

You may overwrite this method if you are interested in directly accessing the multipart stream. Doing so will disable any 'field' / 'file' events processing which would occur otherwise, making you fully responsible for handling the processing.

incomingForm.onPart = function(part) {
  part.addListener('data', function() {
    // ...
  });
}

If you want to use formidable to only handle certain parts for you, you can do so:

incomingForm.onPart = function(part) {
  if (!part.filename) {
    // let formidable handle all non-file parts
    incomingForm.handlePart(part);
  }
}

Taken together, you should be able to do something like this:

function onPart(part) {
    if(!part.filename || part.filename.match(/\.(jpg|jpeg|png)$/i)) {
        this.handlePart(part);
    }
}

app.use(express.bodyParser({onPart: onPart});

Warning: I haven't tested any of this.

Linus Thiel
  • 38,647
  • 9
  • 109
  • 104
  • This is great, Linus, and appears to work. Two questions: (1) Is it better to compare to the content-type vs. the filename? (2) How should I handle not-supported formats? An else { res.send(403) } ? – bento Jul 12 '12 at 19:40
  • I wasn't sure what was available in the `part`, but if you have content-type I would compare with that instead. Regarding how you would do for the `else` case, I'm not sure what options you have. You don't really have access to `res` there, and I don't know how formidable handles that. In my example above, I *think* it will just disregard the file. – Linus Thiel Jul 12 '12 at 20:06
  • Oh and you should probably also do `this.handlePart(part)` for those cases where there is no filename -- I have updated the answer. – Linus Thiel Jul 12 '12 at 20:08
  • Good to know. Formidable appears to just disregard the file when it doesn't match the extension. But it would still be nice to inform the rest of my app about this, and send a response accordingly. – bento Jul 12 '12 at 20:15
2

I found a potential solution:

In your middleware,

    if (req.files[key].type != 'image/png' && req.files[key].type != 'image/jpeg'){
      res.send(403);
    } else {
      next(); 
    }

update: This doesn't actually stop the file from uploading, though.

bento
  • 4,846
  • 8
  • 41
  • 59