ericblade / quagga2

An advanced barcode-scanner written in Javascript and TypeScript - Continuation from https://github.com/serratus/quaggajs
MIT License
766 stars 85 forks source link

get-pixels dependency security issue reported by npm audit with no automatic fix. #494

Open houd1ni opened 1 year ago

houd1ni commented 1 year ago

Hi! There's npm audit fix --force output:

npm audit fix --force
npm WARN using --force Recommended protections disabled.
npm WARN audit No fix available for @ericblade/quagga2@*

up to date, audited 488 packages in 2s

67 packages are looking for funding
  run `npm fund` for details

# npm audit report

request  *
Severity: moderate
Server-Side Request Forgery in Request - https://github.com/advisories/GHSA-p8p7-x288-28g6
No fix available
node_modules/request
  get-pixels  >=2.0.0
  Depends on vulnerable versions of request
  node_modules/get-pixels
    @ericblade/quagga2  *
    Depends on vulnerable versions of get-pixels
    node_modules/@ericblade/quagga2
github-actions[bot] commented 1 year ago

Thank you for filing an issue! Please be patient. :-)

ericblade commented 1 year ago

Thanks for the notice. I'm not the author of get-pixels, so... I don't know?

actually, just had a look at the source code for get-pixels, it's pretty small overall.

Looks like getPixels supports getting pixels from Buffer, dataURL, HTTP/HTTPS, or files.

So it looks like that's to support http/https URLs.

I'm not worried by that vulnerability, as we're not using that functionality. You might appeal to the @get-pixels to get that fixed up .. could easily support node-fetch or just use the internal fetch, or just drop the feature or make it require the user to provide an interface for it..

houd1ni commented 1 year ago

Hi there. I've made a short issue there, pointing here. Is it ok if I make a fork for the lib with this issue fixed if in some time he won't respond ? I really don't want to spoil a terminal with npm warnings to not to miss something important some day.

ericblade commented 1 year ago

Sure, it looks like it would be trivial to remove that dependency from get-pixels, and point quagga at it.

ericblade commented 1 year ago

refer https://github.com/scijs/get-pixels/issues/62#issuecomment-1591909901

looks like he found a usage of getpixels that i wasn't aware of (probably due to some weird capitalization in that file.. . sigh) but i'd still need to further investigate if that is using the questionable part of request.

Commenters advice is sound.

I do not know when I will have time to investigate that. Pull requests always welcome :)