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

Npm security vulnerability in @jupyterlab/rendermime dependency #6479

Closed
SgtPooki opened this issue Jun 5, 2019 · 11 comments · Fixed by #7326
Closed

Npm security vulnerability in @jupyterlab/rendermime dependency #6479

SgtPooki opened this issue Jun 5, 2019 · 11 comments · Fixed by #7326
Labels
pkg:rendermime status:resolved-locked
Milestone

Comments

@SgtPooki
Copy link

@SgtPooki SgtPooki commented Jun 5, 2019

If you are having issues with installation or configuration, we encourage you to post in the Jupyter Discourse forum or file an issue here.

I'm going through the jupyterlab extension tutorial at https://jupyterlab.readthedocs.io/en/stable/developer/xkcd_extension_tutorial.html except i'm using npm instead of jlpm. There is a vulnerability in the package marked that is consumed by @jupyterlab/rendermime

(base) ➜  jupyterlab-ext git:(master) ✗ npm audit
                                                                                
                       === npm audit security report ===                        
                                                                                
┌──────────────────────────────────────────────────────────────────────────────┐
│                                Manual Review                                 │
│            Some vulnerabilities require your attention to resolve            │
│                                                                              │
│         Visit https://go.npm.me/audit-guide for additional guidance          │
└──────────────────────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ Moderate      │ Regular Expression Denial of Service                         │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ marked                                                       │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=0.6.2                                                      │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ @jupyterlab/application                                      │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ @jupyterlab/application > @jupyterlab/docregistry >          │
│               │ @jupyterlab/rendermime > marked                              │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://nodesecurity.io/advisories/812                       │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ Moderate      │ Regular Expression Denial of Service                         │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ marked                                                       │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=0.6.2                                                      │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ @jupyterlab/application                                      │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ @jupyterlab/application > @jupyterlab/rendermime > marked    │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://nodesecurity.io/advisories/812                       │
└───────────────┴──────────────────────────────────────────────────────────────┘
found 2 moderate severity vulnerabilities in 2907 scanned packages
  2 vulnerabilities require manual review. See the full report for details.
@jasongrout
Copy link
Contributor

@jasongrout jasongrout commented Jun 6, 2019

FYI, we've updated master to 0.6.2.

@jasongrout
Copy link
Contributor

@jasongrout jasongrout commented Jun 6, 2019

I'll also note that:

  1. We'll be releasing jlab 1.0 within a few weeks with the upgrade
  2. It appears that the vulnerability is a DOS (i.e., a regex that has quadratic run time), so not as worrying as a vulnerability with ceding permission to an attacker
  3. jlab 0.35.x uses marked 0.4, and upgrading to 0.6.2 for jlab 0.35 be considered a breaking change (i.e., marked changes how it renders markdown in each release, so there are likely changes in md rendering between 0.4->0.6)

@vidartf
Copy link
Member

@vidartf vidartf commented Jun 29, 2019

Closed in 1.0.0?

@joyceerhl
Copy link

@joyceerhl joyceerhl commented Aug 2, 2019

Hey @jasongrout, this is an issue again as of @jupyterlab/rendermime v1.0.1 https://www.npmjs.com/advisories/1076. The patch is now available in >=0.7.0 of marked. Is there any chance master can be updated to 0.7.0?

@jasongrout
Copy link
Contributor

@jasongrout jasongrout commented Aug 2, 2019

I looked at marked 0.7.0, and it looks like their breaking changes either will not affect us (i.e., they are options we don't use), or fix obvious parsing issues. I think I would be okay with marked 0.7.0 going into jlab 1.1, i.e., I don't think it will break things that should work.

@jasongrout jasongrout added this to the 1.1 milestone Aug 2, 2019
@blois
Copy link

@blois blois commented Aug 2, 2019

FWIW, I dug into these some when reporting markedjs/marked#1425. Based on that I would be surprised if regexp backtracking issues did not continue to be an issue for marked.js- the way the parser leverages regular expressions is quite complex.

@jasongrout
Copy link
Contributor

@jasongrout jasongrout commented Aug 2, 2019

Thanks for digging into this. Do you have a suggestion for a better markdown parser?

We considered at one point switching, but ultimately decided on staying with marked for compatibility with the notebook (since it is what the notebook uses). However, as marked is marching inexorably towards commonmark, it may be that we should just switch directly to commonmark and be done.

@blois
Copy link

@blois blois commented Aug 2, 2019

I have not looked at commonmark much but my impression is that the parser is better structured to avoid these issues.

At a glance a switch to commonmark seems daunting, but that may just be from the series of changes from marked.js that we've been slowly absorbing. I don't have a strong recommendation.

@jasongrout
Copy link
Contributor

@jasongrout jasongrout commented Aug 2, 2019

Sorry, when I said switch to commonmark, I meant commonmark the standard, not the reference commonmark parser. The parser we discussed switching to on #272 is markdown-it: https://github.com/markdown-it/markdown-it

@blink1073 blink1073 removed this from the 1.1 milestone Aug 27, 2019
@blink1073 blink1073 added this to the 1.2 milestone Aug 27, 2019
@karrtikr
Copy link

@karrtikr karrtikr commented Oct 3, 2019

Hey @jasongrout, this is an issue again as of @jupyterlab/rendermime v1.0.1 https://www.npmjs.com/advisories/1076. The patch is now available in >=0.7.0 of marked. Is there any chance master can be updated to 0.7.0?

Any update on this? We need this for https://github.com/microsoft/vscode-python/issues/6867 in the Python extension.

@jasongrout
Copy link
Contributor

@jasongrout jasongrout commented Oct 4, 2019

Based on the analysis in #6479 (comment), I think it's okay to upgrade in 1.2, which we are evaluating releasing next week.

Do you want to put in a PR?

@jasongrout jasongrout removed this from the 1.2 milestone Oct 11, 2019
@jasongrout jasongrout added this to the 2.0 milestone Oct 11, 2019
@jasongrout jasongrout added the tag:Backport label Oct 11, 2019
@jasongrout jasongrout removed this from the 2.0 milestone Oct 11, 2019
@jasongrout jasongrout added this to the 1.2 milestone Oct 11, 2019
jasongrout added a commit to jasongrout/jupyterlab that referenced this issue Oct 11, 2019
jasongrout added a commit to jasongrout/jupyterlab that referenced this issue Oct 11, 2019
jasongrout added a commit to jasongrout/jupyterlab that referenced this issue Oct 11, 2019
@jasongrout jasongrout removed the tag:Backport label Oct 12, 2019
@lock lock bot added the status:resolved-locked label Nov 11, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Nov 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pkg:rendermime status:resolved-locked
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants