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

Bump multer to version that removes dicer as sub-dependency #739

Merged
merged 5 commits into from
May 30, 2022

Conversation

finpingvin
Copy link
Contributor

1.4.4 version of multer has a DoS vulnerability through a sub-dependency on dicer. 1.4.4-lts.1 bumps their busboy dependency which in turn drops dicer as a dependency.

#735
https://github.com/expressjs/multer/pull/1097/files#diff-7ae45ad102eab3b6d7e7896acd08c427a9b25b346470d7bc6507b6481575d519

@fgblomqvist
Copy link

fgblomqvist commented May 30, 2022

Have you verified that this works as intended, per this comment: expressjs/multer#1097 (comment) ?
Also, looks like you forgot to update the package-lock.json.

@ghost91-
Copy link

Unfortunately, 1.4.4 is considered to be a higher semver version than 1.4.4-lts.1. So I believe in order for this to work, the version needs to be pinned to 1.4.4-lts.1, i.e. 1.4.4-lts.1, instead of ^1.4.4-lts.1. Not sure why they didn't just publish a 1.4.5, but oh well...

@finpingvin
Copy link
Contributor Author

expressjs/multer#1097 (comment)

I did, but let me recheck. A 1.4.5 would have been so nice..

@finpingvin
Copy link
Contributor Author

You are right, a modified local package-lock made it so 1.4.4-lts.1 was installed. Without it I got 1.4.4.
Published with lockfile that installs 1.4.4-lts.1 but I get a bumped lockfileVersion. I don't know if that is desired. Better to pin the version?

@finpingvin
Copy link
Contributor Author

I will try to downgrade my node and see if I can get a proper lockfile without bumping lockfileVersion.

@finpingvin
Copy link
Contributor Author

With this lockfile it picks the right multer version for me

@LinusU
Copy link

LinusU commented May 30, 2022

I've released 1.4.5-lts.1 which should be considered newer than 1.4.4. We cannot currently release it as 1.4.5 since it contains breaking changes...

@ghost91-
Copy link

ghost91- commented May 30, 2022

With this lockfile it picks the right multer version for me

The problem is: the lockfile is irrelevant for other packages using express-openapi-validator. The test you actually need to perform is having a package depend on your local version of express-openapi-validator, and check how the multer dependency is resolved there.

That said, using the new 1.4.5-lts.1 version should hopefully do the trick, since that should be considered newer than 1.4.4, so when you specify it explicitly (even with ^), npm can’t resolve that to 1.4.4 (which it does if you just specify ^1.4.4-lts.1).

@finpingvin
Copy link
Contributor Author

With this lockfile it picks the right multer version for me

The problem is: the lockfile is irrelevant for other packages using express-openapi-validator. The test you actually need to perform is having a package depend on your local version of express-openapi-validator, and check how the multer dependency is resolved there.

That said, using the new 1.4.5-lts.1 version should hopefully do the trick, since that should be considered newer than 1.4.4, so when you specify it explicitly (even with ^), npm can’t resolve that to 1.4.4 (which it does if you just specify ^1.4.4-lts.1).

Ah of course, stupid me

@cdimascio cdimascio merged commit 6501a62 into cdimascio:master May 30, 2022
@cdimascio
Copy link
Owner

fixed in v4.13.8

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

Successfully merging this pull request may close these issues.

6 participants