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

[huntr-helper]

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 | https://github.com/418sec/node-html-pdf/pull/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 | https://github.com/418sec/node-html-pdf/pull/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 https://github.com/marcbachmann/node-html-pdf/issues/530#issuecomment-536546934, I added as default argument the --local-url-access=false options, which makes impossible reading local files :smile: The user is still able to provide access to the local files setting the readLocalFile option to true :smile:

🐛 Proof of Concept (PoC) *

1. Download the repo
2. Go on the directory node-html-pdf
3. 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' } });

4. `node poc.js`
5. 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 :smile:)

### 👍 User Acceptance Testing (UAT)

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

[JamieSlome]

@marcbachmann - any updates on this?

[huntr-helper]

@marcbachmann - code quality issues addressed and fixed.

[theaccordance]

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

[navjotdhanawat]

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

Thanks

[JamieSlome]

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

[Drarox]

Any updates ?

[clawdaddy]

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

[marcbachmann]

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]

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

[ucefkh]

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

[eran10]

any new here ? can this be merged ?

[olvresc]

Hi, any update on this?

[ucefkh]

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 https://github.com/marcbachmann/node-html-pdf/pull/556#issuecomment-819196360, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAPAQB2CDEBNNRPKQOP255TTIUBPNANCNFSM4KZCKPNA .

--

Youssef KH

[PabloMontoya]

Any news on this?

[marcbachmann]

Fixed in https://github.com/marcbachmann/node-html-pdf/pull/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.