Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
dvtradeling opened this issue Oct 4, 2023 · 6 comments

Comments

@dvtradeling
Copy link

dvtradeling commented Oct 4, 2023

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
Copy link

Can you set the fieldname to an empty string?

@Markiz9999
Copy link

Markiz9999 commented Apr 23, 2024

Is there a workaround to prevent this vulnerability?

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

@joeyguerra
Copy link

Have you tried passing a field name?

@Markiz9999
Copy link

Markiz9999 commented Apr 23, 2024

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
Copy link

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

Issue: #553
Fix: #913

@joeyguerra
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants