search

senny / pdfjs_viewer-rails

PDF.js viewer packaged as a Rails engine.
MIT License
114 stars 175 forks source link

Configurable viewer origins #20

Closed MattFenelon closed 5 months ago

MattFenelon commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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?

senny commented 8 years ago

@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 commented 8 years ago

@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.

MattFenelon commented 8 years ago

@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?

fatuhoku commented 8 years ago

What's the latest on this?