node-html-pdf - Two Arbitrary File Read - Fix #556

huntr-helper · 2020-02-21T12:34:17Z

Fix 1 - Arbitrary File Read

https://github.com/mufeedvh fixed the vulnerability associated with Arbitrary File Read.
This fix is being submitted on behalf of https://github.com/mufeedvh - they have been awarded $25 for fixing the vulnerability through the huntr bug bounty program.
Think you could fix a vulnerability like this - get involved (https://huntr.dev).
Q | A
Version Affected | ALL
Bug Fix | YES
Further References | 418sec#2

Fix 2 - Arbitrary File Read

https://huntr.dev/users/Mik317 fixed the vulnerability associated with Arbitrary File Read.
This fix is being submitted on behalf of https://huntr.dev/users/Mik317 - they have been awarded $25 for fixing the vulnerability through the huntr bug bounty program.
Think you could fix a vulnerability like this - get involved (https://huntr.dev).
Q | A
Version Affected | ALL
Bug Fix | YES
Further References | 418sec#3

User Comments:

📊 Metadata *

Bounty URL: https://www.huntr.dev/bounties/1-npm-html-pdf

⚙️ Description *

The node-html-pdf module is vulnerable against arbitrary file read due to a unsafe usage of phantomjs, which makes possible reading local files when parsing html files to convert in pdf ones.

💻 Technical Description *

As suggested on #530 (comment), I added as default argument the --local-url-access=false options, which makes impossible reading local files 😄
The user is still able to provide access to the local files setting the readLocalFile option to true 😄

🐛 Proof of Concept (PoC) *

Download the repo
Go on the directory node-html-pdf
Create the poc.js file:

var fs = require('fs');
var pdf = require('./');
var html = fs.readFileSync('./test/example.html', 'utf8');
var options = { format: 'Letter' , phantomArgs: ["--web-security=no"]};

pdf.create(html, options).toFile('./example.pdf', function(err, res) {
  if (err) return console.log(err);
  console.log(res); // { filename: '/app/businesscard.pdf' }
});

node poc.js
A example.pdf file with the content of /etc/passwd is created

🔥 Proof of Fix (PoF) *

Same steps with fixed version doesn't create a file with the content of the /etc/passwd file

If the user explicity uses readLocalFile inside the options, then the internal files are fetched again (only who writes the code can change it, so no possible bypasses are allowed for other users 😄)

👍 User Acceptance Testing (UAT)

Run a unit test or a legitimate use case to prove that your fix does not introduce breaking changes.

@mufeedvh

Merging fix - on-behalf of @mufeedvh, executed by huntr.dev (005-js-html-pdf).

JamieSlome · 2020-03-26T06:32:15Z

@marcbachmann - any updates on this?

Addresses code quality issue - https://travis-ci.org/github/marcbachmann/node-html-pdf/jobs/653433658.

huntr-helper · 2020-04-30T09:49:00Z

@marcbachmann - code quality issues addressed and fixed.

theaccordance · 2020-08-25T23:30:02Z

Hi, checking in on the status for accepting this PR, it surfaced in a recent app scan

[FIX] Arbitrary file read using `secure arguments` for `PhantomJS`

navjotdhanawat · 2020-09-08T08:55:29Z

@marcbachmann @huntr-helper when we are planning to publish fix for this issue?

Thanks

JamieSlome · 2020-09-08T10:25:41Z

@navjotdhanawat - we are just waiting to hear back from the maintainer, but hopefully as soon as possible!

marcbachmann

Sorry for the absence.

We can merge that and release it in a major bump as it's a breaking change.
I guess we should also deprecate the library because it's api is phantomjs-oriented and doesn't make sense with more modern setup where a pool of headless workers should be used.

Drarox · 2020-09-30T12:30:56Z

Any updates ?

clawdaddy · 2020-11-09T16:52:01Z

@chriskinsman any chance you could take a look at this?

marcbachmann · 2020-11-09T16:52:57Z

Should we trigger a breaking change here and release v3?
I'd keep the default value to allow file access in v2. But also expose the option to disable it.

JamieSlome · 2020-11-10T12:12:41Z

@marcbachmann - let me know if you need any changes made to the PR, cheers! 👍

ucefkh · 2020-11-10T14:23:49Z

wow nice this is moving, I was about to make a fork and merge this then deploy to npm because this is very serious and needed fix!

Can this be merged today?

thank you guys

marcbachmann · 2020-11-10T14:46:38Z

bin/index.js

@@ -22,7 +27,7 @@ function help () {
 }

 function htmlpdf (source, destination) {
-  var html = fs.readFileSync(source, 'utf8')
+  var html = DOMPurify.sanitize(fs.readFileSync(source, 'utf8'))


this won't help anything regardong security. I wouldn't take jsdom into the project as dependency.
the option which sets the flag is enough. Let's release that as v3 then.
I guess we can can skip a release in v2 as it won't help anything.

I mean we should check whether we can limit the origins of urls (content security policy maybe?). That would be much cleaner. In some cases remote content is desired.

True, normally there should be a restriction to the source or format of origins or urls in the config before even been able to read the file?

Some like

const config = { allowedFolders: ['/tmp','render'], allowedDomains: ['domain1.tld','domain2'] }

I think this would be supported using page.onResourceRequested: https://phantomjs.org/api/webpage/handler/on-resource-requested.html

wow. spot on! then we just match the options in the config with

var match = requestData.url.match(/wordfamily.js/g); or includes? const allowed = config.allowedFolders.contains(requestData.url.split('/')[0])// a bit hacky but you get the idea? if(!allowed) return networkRequest.abort()

So glad y'all are figuring this out. Cause I don't really know what is going on. I just want that npm audit vulnerability to go away.

well this is easy to fix just the incentive to is the issues sometimes :D I can write a good fix for it tho before it goes even to phantomjs

eran10 · 2021-03-18T17:42:07Z

any new here ? can this be merged ?

olvresc · 2021-04-14T03:09:29Z

Hi, any update on this?

ucefkh · 2021-04-14T10:37:05Z

HI Oliver, how are you?

…

On Wed, Apr 14, 2021 at 3:09 AM Oliver Escosura ***@***.***> wrote: Hi, any update on this? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#556 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAPAQB2CDEBNNRPKQOP255TTIUBPNANCNFSM4KZCKPNA> .

-- -------------------- Youssef KH

PabloMontoya · 2021-04-21T23:10:03Z

Any news on this?

marcbachmann · 2021-04-22T00:40:57Z

Fixed in #616 and released with Version 3.

There's a new localUrlAccess option, which sets --local-url-access=false by default.
Sorry for the looong wait. I just don't think it's feasable to maintain this lib as phantomjs got deprecated a long time ago.
I also don't use it anymore and it originally was developed for much simpler use cases than what most people use it for.

mufeedvh and others added 3 commits February 15, 2020 05:23

Added DOMPurify & jsdom to sanitize the HTML input file

e4fe548

Sanitize HTML Input File to prevent JavaScript execution

289fbf7

Merge pull request #2 from mufeedvh/master

648c7d5

Merging fix - on-behalf of @mufeedvh, executed by huntr.dev (005-js-html-pdf).

huntr-helper requested review from chriskinsman and marcbachmann February 21, 2020 12:34

Update index.js

1898ffc

Addresses code quality issue - https://travis-ci.org/github/marcbachmann/node-html-pdf/jobs/653433658.

Mik317 and others added 7 commits August 28, 2020 15:23

[FIX] Arbitrary File Read using --local-url-access=false

74e3e3a

Add readLocalFile options

f9f397f

Update README.md

5b26434

Update README.md

74f554d

Update pdf.js

1a35916

Minor fix

54a1535

Merge pull request #3 from Mik317/master

7e8f6a7

[FIX] Arbitrary file read using `secure arguments` for `PhantomJS`

huntr-helper changed the title ~~node-html-pdf - Arbitrary File Read - Fix:~~ node-html-pdf - Two Arbitrary File Read - Fix Sep 3, 2020

marcbachmann approved these changes Sep 13, 2020

View reviewed changes

marcbachmann reviewed Nov 10, 2020

View reviewed changes

Update pdf.js

d03925e

marcbachmann closed this Apr 22, 2021

node-html-pdf - Two Arbitrary File Read - Fix #556

node-html-pdf - Two Arbitrary File Read - Fix #556

Uh oh!

Conversation

huntr-helper commented Feb 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Fix 1 - Arbitrary File Read

Fix 2 - Arbitrary File Read

User Comments:

📊 Metadata *

Bounty URL: https://www.huntr.dev/bounties/1-npm-html-pdf

⚙️ Description *

💻 Technical Description *

🐛 Proof of Concept (PoC) *

🔥 Proof of Fix (PoF) *

👍 User Acceptance Testing (UAT)

Uh oh!

JamieSlome commented Mar 26, 2020

Uh oh!

huntr-helper commented Apr 30, 2020

Uh oh!

theaccordance commented Aug 25, 2020

Uh oh!

navjotdhanawat commented Sep 8, 2020

Uh oh!

JamieSlome commented Sep 8, 2020

Uh oh!

marcbachmann left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Drarox commented Sep 30, 2020

Uh oh!

clawdaddy commented Nov 9, 2020

Uh oh!

marcbachmann commented Nov 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JamieSlome commented Nov 10, 2020

Uh oh!

ucefkh commented Nov 10, 2020

Uh oh!

marcbachmann Nov 10, 2020

Choose a reason for hiding this comment

Uh oh!

marcbachmann Nov 10, 2020

Choose a reason for hiding this comment

Uh oh!

ucefkh Nov 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marcbachmann Nov 10, 2020

Choose a reason for hiding this comment

Uh oh!

ucefkh Nov 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

clawdaddy Nov 10, 2020

Choose a reason for hiding this comment

Uh oh!

ucefkh Nov 11, 2020

Choose a reason for hiding this comment

Uh oh!

eran10 commented Mar 18, 2021

Uh oh!

olvresc commented Apr 14, 2021

Uh oh!

ucefkh commented Apr 14, 2021 via email

Uh oh!

PabloMontoya commented Apr 21, 2021

Uh oh!

marcbachmann commented Apr 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

huntr-helper commented Feb 21, 2020 •

edited

Loading

marcbachmann left a comment •

edited

Loading

marcbachmann commented Nov 9, 2020 •

edited

Loading

ucefkh Nov 10, 2020 •

edited

Loading

ucefkh Nov 10, 2020 •

edited

Loading

marcbachmann commented Apr 22, 2021 •

edited

Loading