CVE-2022-24434 (issue #882)

[anka-213]

Issue #882 shouldn't have been closed, since it wasn't actually fixed by upgrading multer to 1.4.4. As we can see on https://security.snyk.io/vuln/SNYK-JS-DICER-2311764, there is currently no version of dicer that fixes the issue, so a version bump was not sufficient.

Edit: it seems like the latest version of multer: 1.4.5-lts.1 does fix the issue. However it supports slightly fewer node versions than multer-1.4.4

[anka-213]

not sure how to actually fix the issue though

[anka-213]

Oh, sorry, my mistake, busboy has already removed dicer in its latest version. However, multer doesn't use the latest version yet: expressjs/multer#1122. There is work in progress for that though: expressjs/multer#1097

[attilaorosz]

Thanks for the report! We should keep an eye on multer then.

[rightjelkin]

Hello. Last version of multer v1 (1.4.5-lts.1 or 1.4.4-lts.1) does fix the issue. That is not a mistake. You can see https://github.com/expressjs/multer/blob/v1.4.4-lts.1/package.json there is "busboy": "^1.0.0", that is not using a dicer at all. So it will fix the issue.

It needs just to bump multer to 1.4.4-lts.1

[rightjelkin]

@attilaorosz can you assign it on someone or check? Easy fixable critical thing for improving some security report)

[attilaorosz]

Checking it atm but it causes a test to fail. Investigating.

[attilaorosz]

Seems like busboy >=1.0 dies if you try to submit an invalid multipart form. Even worse, it will drag down your whole app with it. expressjs/multer#1176

I understand that this is critical to solve the vulnerability, but this is a potential app breaking thing.
A possible solution would be to add a precheck middleware and throw an error early.

[attilaorosz]

So it turns out the problem is that headers are not aligned with the payload of the request in the test. If I remove the headers everthing is fine. Frankly, if you mess up your request like that the error message we get instead of the ParamRequiredError is reasonable.

[attilaorosz]

New release should contain the update. Please let me know if it's not sufficient.

[rightjelkin]

I've just checked it. Everything is fine as I can see. Thanks
@anka-213 you can check it too and if it's done I think issue can be closed

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.