CVE-2021-28092 in cssnano due to an old version of is-svg

[prueker]

Describe the bug
CVE-2021-28092

The is-svg package 2.1.0 through 4.2.1 for Node.js uses a regular expression that is vulnerable to Regular Expression Denial of Service (ReDoS). If an attacker provides a malicious string, is-svg will get stuck processing the input for a very long time.

Looking at the dependency tree of css nano shows that postcss-svgo depends on is-svg@3.0.0

└─┬ react-scripts@4.0.3
  └─┬ optimize-css-assets-webpack-plugin@5.0.4
    └─┬ cssnano@4.1.10
      └─┬ cssnano-preset-default@4.0.7
        └─┬ postcss-svgo@4.0.2
          └── is-svg@3.0.0

[alexander-akait]

Please use search before open issue, answered a lot of time, please use rc

[prueker]

Well that's not really an option for most packages since they can't depend on RC's by policy.

Given that cssnano is in the dependency chain of CRA/react-scripts and used by millions of users, a patch of version 4 will likely be required, so keeping this issue open instead of closing the issue and telling people a solution that doesn't work would be more helpful.

[kslimani]

@alexander-akait could you please consider to simply release, for example, a version 4.0.3 of postcss-svgo package (with an upgraded version of is-svg in package.json) ?

It should help to solve the cve issue easier in a lot of projects.

[manooweb]

I agree with the 2 comments above. In addition If I'm not mistaken 5.0.0-rc.2 is not ready yet with the is-svg 4.2.2 patched version
https://github.com/cssnano/cssnano/blob/5.0.0-rc2/packages/postcss-svgo/package.json#L33

Could it be possible to upgrade postcss-svgo package to a patched 4.0.3 version for taking in account the is-svg 4.2.2 version, please ?

I think there is no breaking changes to update to is-svg 4.2.2 version
sindresorhus/is-svg@v3.0.0...v4.2.2

Thanks

[jolting]

Correct. The rc doesn't fix the issue in postcss-svgo, which explains why dependabot created a pr on master. master and 5.0.0-rc.2 are equivalent right now.

[AviVahl]

The 4.x.x branch already uses ^4.1.0: https://github.com/cssnano/cssnano/blob/4.x.x/packages/postcss-svgo/package.json#L33
So it would have caught the new version.

@alexander-akait any chance you could lerna publish --force-publish the 4.x.x branch? It seems as if some of the changes there never got published.

[alexander-akait]

I am afraid v4 can be broken right now, in theory we can found the last commit before publish and use it for update

[AviVahl]

Are there any tests in place? I'm afraid that using specific commits or the rc version is not really an option, as that would require then going project-by-project and getting them all upgraded. Releasing the (already accepted) changes of the 4.x.x branch feels natural to the process, and would target all those use cases already making use of ^4.x.y.

What can be done to verify the soundness of the 4.x.x branch? Any way we can assist?

[AviVahl]

Oh, I see what you mean by specific commit. You mean this: d0f65e2

So if we take this commit + one more bumping the vulnerable dependencies, would that be enough to release a stable 4.x patch?

[alexander-akait]

So if we take this commit + one more bumping the vulnerable dependencies, would that be enough to release a stable 4.x patch?

yep, so we don't break anything

[AviVahl]

Alright, then I suggest the current 4.x.x branch will be deleted. master already contains all those commits. The branch is 32 commits behind master.

Then, create a new 4.x.x branch with all commits up to (including): d0f65e2

Then we'll be able to create a PR from a fork with the required upgrades, directly to the branch.

WDYT?

[alexander-akait]

/cc @ludofischer Let's create branch (v4) from d0f65e2

[ludofischer]

Done https://github.com/cssnano/cssnano/tree/v4

[alexander-akait]

@AviVahl feel free to send a PR, we solve it in v5, let's do the same, just remove is-svg

[ludofischer]

FYI I could not even manage to complete yarn install on the v4 branch with Node.js 14 (some node-gyp errors).

[AviVahl]

I was able to nvm i 10 and yarn. then ran the tests. all passed.
then upgrade to is-svg@4. yarn. yarn test.

got two failures now:

 should reject on malformed svgs


  Expected promise to be rejected, but it was resolved instead

  Resolved with:

  Result {
    css: 'h1{background:url(data:image/svg+xml;charset=utf-8,<svg>style type="text/css"><![CDATA[ svg { fill: red; } ]]></style></svg>)}',
    lastPlugin: Function {
      postcssPlugin: 'postcss-svgo',
      postcssVersion: '7.0.2',
    },
    map: undefined,
    messages: [],
    opts: {},
    processor: Processor {
      plugins: [
        Function { … },
      ],
      version: '7.0.2',
    },
    root: Root {
      indexes: {},
      lastEach: 2,
      nodes: [
        Rule { … },
      ],
      raws: {
        after: '',
        semicolon: false,
      },
      source: {
        input: Input { … },
        start: Object { … },
      },
      type: 'root',
    },
  }



  should not mangle filter effects

  /home/avi/projects/cssnano/packages/postcss-svgo/src/index.js:95

   94:         }           
   95:                     
   96:         return svgo;

  Rejected promise returned by test. Reason:

  Error {
    message: `postcss-svgo: Error in parsing SVG: Text data outside of root node.␊
    Line: 0␊
    Column: 111␊
    Char: #`,
  }

Both, in theory, could be related to is-svg changing behavior to how it deals with some cases. Not sure TBH. Any idea?

[alexander-akait]

@AviVahl let's port this #1034 (no is-svg package)

[AviVahl]

@alexander-akait done. 👍

[alexander-akait]

Done https://github.com/cssnano/cssnano/releases/tag/v4.1.11, hope we will do not break something 😄

[manooweb]

Done https://github.com/cssnano/cssnano/releases/tag/v4.1.11, hope we will do not break something 😄

Thanks 👍

It works for me to get rid of the is-svg vulnerabilty from css-minimizer-webpack-plugin which uses cssnano and because now cssnano doesn't use is-svg package anymore 🤣
But I'm quite sure we don't use SVG in our CSS code. So it isn't significant to say if cssnano@4.1.11 breaks nothing 😁

[ludofischer]

But I'm quite sure we don't use SVG in our CSS code. So it isn't significant to say if cssnano@4.1.11 breaks nothing grin

cssnano can still minify svg, is-svg is an implementation detail. Getting rid of it only changes the way cssnano reports some errors.

[manooweb]

@ludofischer ok thanks for the precision 😉