local file patch

[dr4gonw4ll]

This library is creating temporary files and loading them up with file:/// protocol scheme https://github.com/spatie/browsershot/blob/4148f910735cf385070cb1a6ff1ac26a37eefd10/src/Browsershot.php#L707 when 'Browsershot::html' function is used. Using a file protocol scheme along with headless browser such as Chrome on a server side is not a good idea and will lead to Local file disclosure (Security Vulnerability). Other local files present on the server could be loaded when a malicious HTML is provided. Local files can be loaded either with <iframe> (e.g. <iframe src="file:///etc/passwd" width= height=>) or by setting the different document location. Also, it is unlikely for the developers to check each and every page for malicious JavaScript inputs. This vulnerability can be verified by using below snippet

<?php
use Spatie\Browsershot\Browsershot;
require __DIR__ . '/vendor/autoload.php';
Browsershot::html('<iframe src="file:///etc/passwd" width=500 height=300></iframe>')->save('example.pdf');

I see it can be fixed either in the PHP code or in the JS code. I believe this small JS snippet can be added at https://github.com/dr4gonw4ll/browsershot/blob/4148f910735cf385070cb1a6ff1ac26a37eefd10/bin/browser.js#L171 to make it secure. Below code can be added to library which validates the file protocol scheme and use the 'page.setContent' puppeteer function which loads in 'about:blank' page and does not cause any harm even if malicious JavaScript inputs are provided.

       /**This is a fix for local file access, php creates local temporary file using file scheme protocol. Below code validates
        * the use of file scheme and enforces the usage of page.setContent function
        * refer https://github.com/puppeteer/puppeteer/blob/v5.4.1/docs/api.md#pagesetcontenthtml-options
        */
        if(request.url.trim().toLowerCase().substring(0,4)=='file'){
            await page.setContent(fs.readFileSync(new URL(request.url),'utf-8'),requestOptions);
        }
        else
        {
            await page.goto(request.url, requestOptions);
        }

I am not sure about the performance impact this additional code will create. Let me know your comments

[freekmurze]

Hi, I'd rather have this fixed on the PHP side. Please also add a test to make sure that the fix works correctly.

[dr4gonw4ll]

Hello, I did take a look into the PHP code, I though I could use data:text/html;base64,<encoded content> URI as the chrome API page.goto supports it. However, this method with data URI would take more time and call gets timed out. I think using this technique would also effect the performance. The other way to fix in PHP code would be to block or filter the file:// URI schemes which is not the suggestable way.

I see fixing the code in browser.js and using the page.setContent is the best option.

[freekmurze]

Thanks!