pdfjs_viewer-rails icon indicating copy to clipboard operation
pdfjs_viewer-rails copied to clipboard

Configurable viewer origins

Open MattFenelon opened this issue 9 years ago • 6 comments

Closes #16.

I've changed the implementation to a solution that doesn't involve editing the viewer.js file.

The HOSTED_VIEWER_ORIGINS can now be configured in an initialiser:

PdfjsViewer.hosted_viewer_origins = ["http://somehost", "https://somehost"]

MattFenelon avatar Aug 03 '16 17:08 MattFenelon

Thinking about it more, the file parameter is a security risk for content spoofing when HOSTED_VIEWER_ORIGINS is configured to let the viewer load any remote URL. Consider the case where pdfjs_viewer-rails is hosted on open-to-attack.com and HOSTED_VIEWER_ORIGINS is set to ["http://open-to-attack.com"], a malicious person could send a link to another person, such as http://open-to-attack.com/pdfjs/file=, with file set to any remote pdf url. The HOSTED_VIEWER_ORIGINS doesn't protect against this, nor does CORS which is only concerned with the requested URLs headers.

Maybe pdf.js needs another check against what URLs it'll load from, or pdfjs_viewer-rails shouldn't expose the file parameter on the viewer, loading PDFs in a different way perhaps.

MattFenelon avatar Aug 04 '16 11:08 MattFenelon

The Content-Security-Policy header could be used to protect sites from loading any remote URL. A configuration option could be made available to set that header on requests for any of pdfjs_viewer-rails views?

MattFenelon avatar Aug 04 '16 11:08 MattFenelon

@MattFenelon since the request from the viewer is always going to be a GET maybe it's not critical. I think it's something that should be left to the PDF.js project. If possible this gem should do as little as necessary to bundle that library. Otherwise updating with the latest PDF.js version will get very painful.

senny avatar Aug 08 '16 12:08 senny

@MattFenelon I still don't quite understand why PDF.js chose to implement the check that way. Why would a hosted viewer be good to load all remote urls but one on the same host not. Somehow these things feel unrelated except that a hosted viewer might be very likely to load remote PDFs.

senny avatar Aug 08 '16 15:08 senny

@senny there's a FAQ entry about it, as far as I can tell it's purpose is to avoid opening a security hole on your site inadvertently, if you change the code to remove the check you've at least had to accept the risk of doing so. There's also some details in https://github.com/mozilla/pdf.js/pull/6916.

We could add a warning to the README about changing the PdfjsViewer.hosted_viewer_origins setting, and potentially mention using CSP to protect against loading any old remote PDF, what do you think? Or are you against this PR altogether?

MattFenelon avatar Aug 10 '16 10:08 MattFenelon

What's the latest on this?

fatuhoku avatar Oct 13 '16 12:10 fatuhoku