npm audit vulnerability

[antoniovassell]

Hey there, an npm advisory is out for this package. https://www.npmjs.com/advisories/1095

Is there any timeline to resolve this?

[kristoff2016]

Thanks for great plugin. Do you have any plan fix it sooner? thank you

[antoniovassell]

Seems puppeteer will be the answer. Not much other well supported choices for html to pdf converters.

[278kunal]

Any alternatives for this packages apart from the puppeteer approach ?

[antoniovassell]

@278kunal There is json to pdf. https://www.npmjs.com/package/pdfkit which is very popular. You would have to convert your existing templates from html to json but it works.

[marcbachmann]

I'm not sure why you can't just provide the phantomjs argument using the config phantomArgs: ['--local-url-access=false']

[antoniovassell]

@marcbachmann Could you explain how this would help with the npm vulnerability above? https://www.npmjs.com/advisories/1095

[sin6pi7]

I decided to replicate potential attacks if possible. In order to do that I played with phantomjs arguments (https://phantomjs.org/api/command-line.html). Below you can find the experiment and accompanying results.

Preparation

0. Clone the repo,
1. cd examples,
2. echo "private content" > private,
3. Add script adding contents of local file into the businesscard/businesscard.html file (right before the </body> tag):

    <div id="file" style="color: white"></div>
    <script>
        const el = document.getElementById('file');
        function reqListener () {
            el.innerHTML = this.responseText;
        }

        var oReq = new XMLHttpRequest();
        oReq.addEventListener("load", reqListener);
        oReq.open("GET", "{{path}}");
        oReq.send();
    </script>

4. Pass the path to private file when rendering from template i.e. replace in serve-http/index.js.

const html = tmpl.replace('{{image}}', `file://${require.resolve('../businesscard/image.png')}`);

with

  const html = tmpl
    .replace('{{image}}', `file://${require.resolve('../businesscard/image.png')}`)
    .replace('{{path}}', `file://${require.resolve('../private')}`);

Test

Rendering from web server with default phantomArgs

Start web server:

$ node serve-http/index.js
Listening on http://localhost:8080

Go to http://localhost:8080 in your browser. Look at the output from console.

NETWORK_ERR: XMLHttpRequest Exception 101: A network error occurred in synchronous requests.

  undefined:107 in send

=> Cross-origin request was not allowed. Private content not visible in the rendered pdf.

Rendering from web server with web security turned off (--web-security=no)

Update serve-http/index.js line 9 to:

pdf.create(html, {width: '50mm', height: '90mm', phantomArgs: ['--web-security=no']}).toStream((err, stream) => {

Start web server:

$ node serve-http/index.js
Listening on http://localhost:8080

Go to http://localhost:8080 in your browser and observe private content in response. => Cross-origin request was allowed. Private content visible in the rendered pdf.

Rendering from web server with web security turned off and access to local files turned off (--web-security=no --local-url-access=false)

Update serve-http/index.js line 9 to:

pdf.create(html, {width: '50mm', height: '90mm', phantomArgs: ['--web-security=no', '--local-url-access=false']}).toStream((err, stream) => {

Start web server:

$ node serve-http/index.js
Listening on http://localhost:8080

=> Request for local file was not allowed. Private content not visible in the rendered pdf. However, since the image was loaded via FS url it's not loaded either.

Rendering from CLI with default phantomArgs

Replace ../bin/index.js line 25 with:

  var html = fs
    .readFileSync(source, 'utf8')
    .replace('{{image}}', `file://${require.resolve('../examples/businesscard/image.png')}`)
    .replace('{{path}}', `file://${require.resolve('../examples/private')}`);

Run:

$ ../bin/index.js businesscard/businesscard.html test.pdf

=> Request for local file was allowed. Private content visible in the rendered pdf.

Rendering from CLI with access to local files turned off (--local-url-access=false)

Replace ../bin/index.js line 31 with:

  base: 'file://' + path.resolve(source), phantomArgs: ['--local-url-access=false'],

Run:

$ ../bin/index.js businesscard/businesscard.html test.pdf

Request for local file was not allowed. Private content not visible in the rendered pdf.

Conclusions

According to the above tests looks like suggestion from @marcbachmann to use --local-url-access=false phantomjs argumnet succesfully prevents loading files from local file system using XHR requests. However, I'm not sure if this is the only attack vector NPM researchers had in mind, so it is not, in any way, a confirmation the vulnerability does not exist.

[antoniovassell]

Hi @sin6pi7 the last post was really informative, thanks for putting this together. I figure that suggestion from @marcbachmann would prevent such an attack. However what I am wondering is, given that I, and I suspect other people also, have npm audit apart of their CI/CD pipeline. This deployment step is currently failing because npm audit fails.

Do you guys turn off npm audit or not have it apart of your CI/CD pipeline? @sin6pi7 @marcbachmann ? What do you generally do?

Thanks again for your time.

[sin6pi7]

Thanks @antoniovassell, appreciated 👍

I figure that suggestion from @marcbachmann would prevent such an attack.

Please remember, that this only tackles the XHR for local files reported in the original description on npm website - might be that npm researchers found other attack vectors, which I have not covered. Suggestion for disabling local files access in phantomjs should be evaluated against your own use case.

Do you guys turn of npm audit or not have it apart of your CI/CD pipeline? @sin6pi7 @marcbachmann ? What do you generally do?

Yeah, I've been there and decided to use https://www.npmjs.com/package/npm-audit-resolver - it allows you to choose whether you would like to ignore something if you're making an informed decision.

[antoniovassell]

Hi @sin6pi7 , npm-audit-resolver looks like a great option for that. Thanks for the insights.

[jfaylon]

Sadly, we don't have the privilege of choosing alternatives to npm audit in our CI. Is there a way to mitigate this other than choosing other libraries? Or is there an alternative that only requires minimal changes to the templates initially processed by this library?

Thanks :)

[crazyoptimist]

In your package.json file, replace "html-pdf": "^2.2.0" with

"html-pdf": "git+https://github.com/418sec/node-html-pdf.git"

You can try this until a new patch would be published, that repo seems to be safe to use imo.

[snowmac]

In your package.json file, replace "html-pdf": "^2.2.0" with

"html-pdf": "git+https://github.com/418sec/node-html-pdf.git"

You can try this until a new patch would be published, that repo seems to be safe to use imo.

Can we get this published on NPM? That would be great. Love what you did there.

[crazyoptimist]

Can we get this published on NPM? That would be great. Love what you did there.

It's not my repo, I just found it, @snowmac Ask the owners maybe?

[snowmac]

@418sec can you publish to NPM?

[jt-turner-upkeep]

Can @marcbachmann merge and publish the change? If not maybe we need to fork the repo and rename it so we can publish it.

[marcbachmann]

I have the fix in #616, just phantomjs fails to run on the ci. It's working locally.

[jt-turner-upkeep]

Thank you @marcbachmann

[fakelag]

There is a fix in 3.0.1 now but the advisory still lists it as affected https://www.npmjs.com/advisories/1095

[RopoMen]

I contacted NPM support and this advisory 1095 is now fixed in the 3.0.1 version https://www.npmjs.com/advisories/1095/versions

[karlhorky]

@antoniovassell can this be closed now?

[Ellosaur]

Its still listing as an issue in version 3.0.1 in tools like Meterian The github advisory seems to contradict itself saying that 3.0.1 patches the issue but also stating: "No fix is currently available".