Npm security vulnerability in @jupyterlab/rendermime dependency

[SgtPooki]

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]

FYI, we've updated master to 0.6.2.

[jasongrout]

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]

Closed in 1.0.0?

[joyceerhl]

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]

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.

[blois]

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]

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]

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]

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

[karrtikr]

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]

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?