expressjs / multer

Node.js middleware for handling `multipart/form-data`.
MIT License
11.57k stars 1.06k forks source link

TypeError: Cannot read properties of undefined (reading 'length') when fieldNameSize is not set #1233

Open dvtradeling opened 12 months ago

dvtradeling commented 12 months ago

We use Nest.js FilesInterceptor which uses multer under the hood.

When fieldname is not provided, but fieldNameSize is set, the above line is still executed and TypeError is thrown accordingly. Check this - https://github.com/expressjs/multer/blob/master/lib/make-middleware.js#L103

We need to prevent it.

Screenshot 2023-10-04 at 12 39 44

For fixing this issue please add this line

Screenshot 2023-10-04 at 12 53 18
joeyguerra commented 6 months ago

Can you set the fieldname to an empty string?

Markiz9999 commented 5 months ago

Is there a workaround to prevent this vulnerability?

In my case, sending such a request to the express server crashes the server.

joeyguerra commented 5 months ago

Have you tried passing a field name?

Markiz9999 commented 5 months ago

Sure. With the passed field name everything works fine. But the important thing here is to prevent the application from crashing when the client does something illegal (in this case, send a request with an empty field name).

If your question is did I try to send an empty string in the field name, then by default it is an empty string, but in that case the busboy library emits an undefined value.

https://github.com/mscdex/busboy/blob/master/lib/types/multipart.js#L296

partName = undefined;

https://github.com/mscdex/busboy/blob/master/lib/types/multipart.js#L313

if (disp.params.name)
  partName = disp.params.name;

https://github.com/mscdex/busboy/blob/master/lib/types/multipart.js#L358

this.emit(
  'file',
  partName,
  this._fileStream,
  { filename,
    encoding: partEncoding,
    mimeType: partType }
);
Markiz9999 commented 5 months ago

Previously, the same case was fixed for fields. So we need the same check for files.

Issue: https://github.com/expressjs/multer/issues/553 Fix: https://github.com/expressjs/multer/pull/913

joeyguerra commented 5 months ago

I see. Just need to get the same fix in. Got it.