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

[6 High Vuln] Bump to remark-parse@9.0.0 #312

Closed
codejedi365 opened this issue May 22, 2021 · 2 comments
Closed

[6 High Vuln] Bump to remark-parse@9.0.0 #312

codejedi365 opened this issue May 22, 2021 · 2 comments
Labels
📦 area/deps This affects dependencies 🏡 area/internal This affects the hidden internals 🔒 area/security This affects security external known issue from external, can't do much by ourselves 👯 no/duplicate Déjà vu 🙋 no/question This does not need any changes 🙅 no/wontfix This is not (enough of) an issue for this project 💬 type/discussion This is a request for comments

Comments

@codejedi365
Copy link

codejedi365 commented May 22, 2021

Subject of the issue

Installation creates 6 high vulnerabilities within the project scope which can be avoided since there is a patched solution.

Problem

The derived dependency on trim@<=0.0.3 which has a high severity vulnerability specifically Regular Expression Denial of Service. This package is relied upon by remark-parse@8.0.3 which is what eslint-plugin-mdx package uses in src/rules/helpers.ts.

This problem cannot be auto-fixed due to the semver major change within remark-parse and for trim itself. NPM will not resolve it without developer intervention. Do not be fooled by npm being able to "fix". It will not actually work since it tries to resolve by downgrading to eslint-plugin-mdx@1.4.2 instead and then further downgrades but still all rely on trim or have other vulnerabilities.

Solution

Major version bump of remark-parse to v9.0.0. In remark-parse@v9.0.0, the author removed the dependency on trim completely and moved to a separate utility suite for markdown specifically which has 0 known/reported vulnerabilities at this time.

CAVEAT: This will only resolve 2 of the 6 listed vulnerabilities from npm audit until the eslint-plugin-markdown#188 pull request is resolved. eslint-plugin-mdx uses eslint-plugin-markdown as a dependency which is why the vulnerability listing will still have 4 remaining. If the resolution version ends up being a major version change for eslint-plugin-markdown then this repository will need to be updated too. A minor semver change will require users to run npm audit --fix --force after installation but if it is only an update then npm should update automatically for new users or manually through npm audit --fix for previous users.

Your environment

  • OS: Mac OS Big Sur (v11.3.1)
  • Packages: eslint-plugin-mdx
  • Env: node v14.16.1, npm 7.13.0

Steps to reproduce

  1. In a npm initialized project, runnpm install eslint-plugin-mdx which installs v1.13.0 from npmjs.com
    npm install runs npm audit by default but produces limited report.
  2. Run npm audit
  3. To better understand where these are coming from, run npm list trim to see the packages that state trim@<=0.0.3 as a dependency.

Expected behaviour

What should happen? npm audit resolves to "0 vulnerabilities".

OR

until the caveat is resolved at least down to 4 vulnerabilities listed. With the bump 2 should be resolved. NPM@7 uses npm-audit-report@v2.0.1 which counts one metaVulnerability for each library which depends on a vulnerable package.

Actual behaviour

What happens instead? npm audit spits out "6 high severity vulnerabilities" upon audit.

@codejedi365 codejedi365 added 🐛 type/bug This is a problem 🙉 open/needs-info This needs some more info labels May 22, 2021
@JounQin
Copy link
Member

JounQin commented May 23, 2021

remark-parse@9.0.0 is BREAKING CHANGE, its support is part of eslint-mdx@2 which is not available yet.

duplicate of mdx-js/mdx#1458, mdx-js/mdx#1531, mdx-js/mdx#1548, mdx-js/mdx#1553

@JounQin JounQin closed this as completed May 23, 2021
@JounQin
Copy link
Member

JounQin commented May 23, 2021

I don't know how to resolve it in npm, but for yarn, you can just try the following for now:

// package.json
{
  "resolutions": {
    "trim": "^1.0.1"
  }
}

@JounQin JounQin added 👯 no/duplicate Déjà vu external known issue from external, can't do much by ourselves 🏡 area/internal This affects the hidden internals 💬 type/discussion This is a request for comments 📦 area/deps This affects dependencies 🔒 area/security This affects security 🙋 no/question This does not need any changes 🙅 no/wontfix This is not (enough of) an issue for this project and removed 🐛 type/bug This is a problem 🙉 open/needs-info This needs some more info labels May 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📦 area/deps This affects dependencies 🏡 area/internal This affects the hidden internals 🔒 area/security This affects security external known issue from external, can't do much by ourselves 👯 no/duplicate Déjà vu 🙋 no/question This does not need any changes 🙅 no/wontfix This is not (enough of) an issue for this project 💬 type/discussion This is a request for comments
Development

No branches or pull requests

2 participants