search

richardgirges / express-fileupload

Simple express file upload middleware that wraps around busboy
MIT License
1.52k stars 261 forks source link

Passing a Buffer body will pollute req.body when used along with `processNested` #290

Closed jeanbmar closed 2 years ago

jeanbmar commented 2 years ago

Proxies tend to pass multipart body as Buffer instances to express. (e.g. @vendia/serverless-express and serverless-http).

This implementation will cause a disaster when the provided body is a Buffer and imo could lead to possible security issues: https://github.com/richardgirges/express-fileupload/blob/40d027fc978d9353da7abcc20faa010761074dd9/lib/processNested.js#L8-L9 Each byte of the multipart data buffer is becoming a key on the body:

body {
  '0': 45,
  '1': 45,
  '2': 45,
  '3': 45,
  '4': 45,
  '5': 45,
  '6': 87,
  '7': 101,
  '8': 98,
  '9': 75,
  '10': 105,
  '11': 116,
  [...]
  name: 'Tset2',
  slug: 'tset',
}

I suggest creating a body from scratch to avoid this instead of reusing the body provided by express, as Multer does.

jmikrut commented 2 years ago

Hey @richardgirges — looks like this has been merged in, but I'm not seeing a new release that includes it. Will you be publishing a new version?

Thanks!

richardgirges commented 2 years ago

Hi @jmikrut - this has been released in v1.2.1+.

I apologize, as the release notes did not have #291 listed. I've updated the release notes to reflect this update: https://github.com/richardgirges/express-fileupload/releases/tag/v1.2.1

If you're running on Node v12+, I would recommend upgrading to the latest version (v1.3.1) to take advantage of the latest security patches.