Upload security

[expressjs]

Upload security

[jpfluger]

This was a thoughtful post. If multer is used globally, multer is exposed to these vulnerabilities. But if the middleware is placed directly on the route, the security concerns are mitigated. I just wrote a doc explaining how to do this.

[mdaffin]

Not entirely, you still have to check all the files in req.files and delete any you are not expecting. I have seen quite a few examples that only check for req.files.images, for example, with no limit set (I have not seen any set a limit) which means an attacker could upload a file to req.files.bad to bypass any check made. Even with a limit set you could just upload to req.files.bad and watch the code return an error as you did not set req.files.images... but by this point your file is already on the server. Unless you check all files in req.files.images and set multer middle ware on only the expect routes you are vulnerable.

[jpfluger]

Yes, I see now. I've setup some tests and verified this. As a side-effect, one of the tests showed that surpassing the file limit creates a condition where multer never "finishes" the current process of parsing files: so the server hangs. I'm gonna try to fix this now and then add in support for "explicitFields." I already have a pull request to add in-memory parsing (no automatic write-to-disk). So hopefully these can get merged in or else I may clutter up the code. When I get my tests done, I'll drop a message on this thread to get your thoughts if what I'm looking at is right/wrong/needs nudging.

[jpfluger]

I added support for fieldNameSize limitations in busboy and initiated a pull request. Once that lib gets updated, we can work through hardening multer and will report back here.

[hacksparrow]

👍

[LinusU]

@James147 I believe this to be solved by the newly released version 1.0.0. Please reopen if you have any further comments on how we could improve the security.