jimp-dev / jimp

An image processing library written entirely in JavaScript for Node, with zero external or native dependencies.
http://jimp-dev.github.io/jimp/
MIT License
13.86k stars 760 forks source link

npm audit fails about xml2js (from load-bmfont) #1223

Closed edi9999 closed 1 week ago

edi9999 commented 1 year ago

On my project, I am using jimp, and just found out that the current latest version : 0.22.7, has a vulnerable dependency.

Here is the output of npm audit :

xml2js  <0.5.0
Severity: high
xml2js is vulnerable to prototype pollution  - https://github.com/advisories/GHSA-776f-qx25-q3cc
fix available via `npm audit fix --force`
Will install jimp@0.3.5, which is a breaking change
node_modules/xml2js
  parse-bmfont-xml  *
  Depends on vulnerable versions of xml2js
  node_modules/parse-bmfont-xml
    load-bmfont  >=1.1.0
    Depends on vulnerable versions of parse-bmfont-xml
    node_modules/load-bmfont
      @jimp/plugin-print  *
      Depends on vulnerable versions of load-bmfont
      node_modules/@jimp/plugin-print
        @jimp/plugins  *
        Depends on vulnerable versions of @jimp/plugin-print
        node_modules/@jimp/plugins
          jimp  >=0.3.6-alpha.5
          Depends on vulnerable versions of @jimp/plugins
          node_modules/jimp

6 high severity vulnerabilities

To address all issues (including breaking changes), run:
  npm audit fix --force

It seems that @jimp/plugin-print uses :

There is an issue in parse-bmfont-xml to upgrade to xml2js 0.5.0 : https://github.com/mattdesl/parse-bmfont-xml/issues/6

lorand-horvath commented 1 year ago

A quick and dirty solution until parse-bmfont-xml bumps xml2js to 0.5.0 is to add an override to your package.json https://github.com/Leonidas-from-XIV/node-xml2js/issues/671#issuecomment-1516405738

  "overrides": {
    "jimp": {
      "xml2js": "^0.5.0"
    }
  }

and npm install.

Note: overrides are only available since npm 8.3

zmedgyes commented 1 year ago

An alternative / temporary solution could be to create a custom-configured Jimp by using @jimp/custom , if you know, you are not using features from the @jimp/plugin-print plugin. With that you can eliminate the problematic dependecy manually. Please note, that you might run into some issues with that, if you are using Typescript (depending on your configuration, I made PR where I try to make the modular configuration more available: https://github.com/jimp-dev/jimp/pull/1225 ).

blastshielddown commented 1 year ago

FWIW I'm now seeing this vulnerability categorized as moderate. Not sure what has changed or if npm audit works differently on different node versions. I'm on v16.20.0.

lorand-horvath commented 1 year ago

It's high time for parse-bmfont-xml to be updated to include the latest xml2js 0.6.0 https://github.com/Leonidas-from-XIV/node-xml2js/tags but there's no movement in any of the already existing PRs: https://github.com/mattdesl/parse-bmfont-xml/pull/4 https://github.com/mattdesl/parse-bmfont-xml/pull/5

If this won't progress, would the jimp devs fork it or find an alternative, please ? @hipstersmoothie @Marsup @zmedgyes @sjoerd108

hipstersmoothie commented 1 year ago

If someone want to fork those deps into the jimp org and do the update I'll help make it happen

pzrq commented 1 year ago

A quick and dirty solution until parse-bmfont-xml bumps xml2js to 0.5.0 is to add an override to your package.json Leonidas-from-XIV/node-xml2js#671 (comment)

  "overrides": {
    "jimp": {
      "xml2js": "^0.5.0"
    }
  }

and npm install.

Note: overrides are only available since npm 8.3

The nested form did not work for me in npm 9.6.3, though the following did reliably generate the upgraded non-vulnerable package-lock.json I needed on npm install:

  "overrides": {
    "xml2js": "^0.5.0"
  }
lorand-horvath commented 1 year ago

@pzrq The nested form works perfectly fine. But there's a catch. Whenever you install a new package, e.g. npm install some-package the defined override will not be taken into account at all, so the old xml2js will be reinstalled. This is some very strange quirk of npm and many people have run into this issue. The solution is to delete node_modules and package-lock.json and then do a clean npm install of all packages. This will apply the override as expected. I'm not sure if this is what you have run into, but most certainly many are struggling with this, still.

isahann commented 1 year ago

Is there any progress available on the work in this vulnerability? I'm having the same issue here and I'm using v0.22.10.

edi9999 commented 12 months ago

No, same issue for me as of now.

RazvanVuscan commented 11 months ago

This issue is still occurring for me as well.

lorand-horvath commented 11 months ago

Very strange that nobody bothers to actually fix this https://github.com/jimp-dev/jimp/issues/1223#issuecomment-1587654554

RazvanVuscan commented 11 months ago

@lorand-horvath from your original workaround, I've tried adding the override to my package.json , deleted package-lock.json and node_modules, but I still see the vulnerability during the yarn audit process. Seems v. 0.4.5 keeps getting pulled in.

Am I doing something wrong?

lorand-horvath commented 11 months ago

@RazvanVuscan As per https://github.com/jimp-dev/jimp/issues/1223#issuecomment-1516413127 : In package.json add this, which will override xml2js to the currently latest version 0.6.2:

  "overrides": {
    "jimp": {
      "xml2js": "^0.6.0"
    }
  }

Then delete package-lock.json and node_modules. Then npm install. This will only work if you have npm 8.3 or newer - you can check with npm -v. This should work. If not, what version of node do you have? node -v

Edit: I see you're using yarn instead of npm. I haven't worked much with it. I'm not sure if and since what version of yarn are overrides supported, you'd have to dig a bit and find out. If you do, please let me know!

RazvanVuscan commented 11 months ago

@lorand-horvath yeah, I actually use nvm, and my node version is 20.0.0. And yes, I use yarn instead of npm because of speed reasons 😄 .

But your suggestions did provide me a solution, and as per https://classic.yarnpkg.com/en/docs/selective-version-resolutions/#toc-how-to-use-it.

If you are using yarn, add this to your package.json for a quick and dirty solution:

    "resolutions": {
        "jimp/@jimp/plugins/@jimp/plugin-print/load-bmfont/parse-bmfont-xml/xml2js": "^0.6.0"
    }

yarn uses resolutions not overrides 😄 .

benmccann commented 11 months ago

Perhaps jimp should not install all plugins automatically? I have no need to print bitmap fonts on my files, but plugin-print is automatically included. It feels like something that should be made into an optional peer dependency

RonaldPM commented 8 months ago

Seems like issue is causing security tools such as Snyk to flag Jimp. Is there a plan for a fix yet?

lorand-horvath commented 7 months ago

Fixed in https://github.com/mattdesl/parse-bmfont-xml/pull/4

benmccann commented 7 months ago

Hurray! There's nothing left to do here as users can now update their lockfiles without any changes necessary in jimp

That being said, I do think it'd be an improvement if all jimp plugins were not automatically installed: https://github.com/jimp-dev/jimp/issues/1223#issuecomment-1755675763

hipstersmoothie commented 6 months ago

If anyone wants to submit a pr to update our deps I'd approve! not installing all the default plugins is a pretty big breaking change and I don't think breaking changes are too worth it for this project

hipstersmoothie commented 1 week ago

:rocket: Issue was released in v1.0.2 :rocket: