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

Security Issues in dicer package #1512

Closed
Uzlopak opened this issue Dec 5, 2021 · 18 comments · Fixed by #1757
Closed

Security Issues in dicer package #1512

Uzlopak opened this issue Dec 5, 2021 · 18 comments · Fixed by #1757
Assignees

Comments

@Uzlopak
Copy link

Uzlopak commented Dec 5, 2021

Hi,

we forked busboy and fixed two critical bugs in the package, which could cause the node-process to crash or to hang. Those bugs originated in the dicer package, which we integrated in our busboy fork and, as I already mentioned, fixed. Is there any interest in a separate @fastify/dicer package, were we only ship the bugfixed dicer package?

for tracking reasons:
fastify/busboy#68

@google-oss-bot
Copy link

I found a few problems with this issue:

  • I couldn't figure out how to label this issue, so I've labeled it for a human to triage. Hang tight.
  • This issue does not seem to follow the issue template. Make sure you provide all the required information.

@lahirumaramba
Copy link
Member

lahirumaramba commented May 20, 2022

Hi @Uzlopak Thank you for reaching out. Apologies for not responding to this earlier. Do you know if fastify is still interested in shipping a bug fixed dicer package?

@harshagarwal00
Copy link

harshagarwal00 commented May 23, 2022

@lahirumaramba :
I am also getting stuck because of snyk issues with dicer..
If we can migrate to fastify/busboy or fastify/dicer will be great!!

fastify/busboy team is v open to create a fastify/dicer if we need it...
Reference: kibertoad comment in mscdex/dicer#22

busboys fix: fastify/fastify-multipart#297

@samarpanB
Copy link

We are also facing the same issue. We are blocked for deployment on production environment because of npm audit failing and snyk also failing. Reason is this issue only. How soon can we fix it ?

@Uzlopak
Copy link
Author

Uzlopak commented Jun 1, 2022

We did not plan to create a separate fork of dicer. But I could export dicer class from our busboy-fork.

@lahirumaramba
Copy link
Member

lahirumaramba commented Jun 2, 2022

@Uzlopak thank you! I think that would be the fastest way to address this issue. Do you know what your current release schedule is? I am also looking into replacing dicer with busboy, though I am not sure if that would be an overkill for the current use case we have in the admin sdk.

@Uzlopak
Copy link
Author

Uzlopak commented Jun 2, 2022

@kibertoad
Can we tomorrow do this?

@kibertoad
Copy link

@Uzlopak sure, I don't see why not.

@samschurter
Copy link

We are running into this security warning on several of our repos. Thanks for working on this!

@rafaelmaeuer
Copy link

any news on this?

@Uzlopak
Copy link
Author

Uzlopak commented Jun 7, 2022

Exports dicer from fastify busboy

fastify/busboy#90

As busboy is already default import, it would be necessary to use the named export Dicer

@harshagarwal00
Copy link

brilliant! :D

@samschurter
Copy link

@lahirumaramba Now that there is a fixed fork of Dicer available, do you need someone to open a PR with a fix here or are you planning on doing that?

@Uzlopak
Copy link
Author

Uzlopak commented Jun 7, 2022

@kibertoad
Can you please make a release?

@kibertoad
Copy link

will do tomorrow

@lahirumaramba
Copy link
Member

Thank you @Uzlopak , @kibertoad !
Once the new release is out we will update the dependencies and release firebase-admin with the fix.

@kibertoad
Copy link

@lahirumaramba @fastify/busboy 1.1.0 with exported fixed Dicer is out

@lahirumaramba
Copy link
Member

Many thanks to @Uzlopak and @kibertoad ! This should be fixed in firebase-admin v10.3.0. Thank you for your patience, everyone!

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

Successfully merging a pull request may close this issue.

8 participants