Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Interop: pdf might or might not render in a sandboxed iframe (depending on a browser) #3958

Closed
anforowicz opened this issue Aug 23, 2018 · 11 comments · Fixed by #6946
Closed
Labels
interop Implementations are not interoperable with each other topic: embed and object topic: sandbox

Comments

@anforowicz
Copy link

Iframe sandbox disables plugins. This leads to the following interop problem: some browsers (like Chrome) implement PDF rendering via a plugin and therefore PDF might not render in a sandboxed iframe (or in pop-ups opened from a sandboxed iframe).

Brainstorming some ideas for how the spec can help interoperability here:

  • Maybe the spec can introduce a mechanism for more granular sandboxing of plugins (e.g. so that a parent frame may allow its subframe to embed plugins handling application/pdf but not application/x-shockwave-flash). This doesn't seem to help interop, because for backcompatibility, the default behavior would have to block all plugins (and so the default behavior would remain not interoperable)

  • Maybe the spec can give browsers some leeway in what plugins are blocked. For example - Chrome's PDF rendering restricts which APIs are exposed to PDFs (e.g. Net.* methods are not implemented) and maybe such restriction means that sandboxed frames should be allowed to embed PDF plugin.

  • In the long term high-profile plugins like Flash and PNaCl will be deprecated (Flash by 2020 although AFAIK PNaCl remains supported for Chrome Apps for foreseeable future). Maybe this means that browsers shouldn't render PDF via a plugin?

Other notes:

@anforowicz
Copy link
Author

FYI: @domenic, @tsepez

@domenic
Copy link
Member

domenic commented Aug 23, 2018

/cc @bzbarsky and @travisleithead as people who I remember being interested in issues around PDFs as a very special type of "plugin" (sometimes not a plugin). And @rniwa for good measure.

Vaguely related: #3462

@domenic domenic added the interop Implementations are not interoperable with each other label Aug 23, 2018
@annevk
Copy link
Member

annevk commented Aug 24, 2018

https://html.spec.whatwg.org/#sandboxed-plugins-browsing-context-flag already allows for exceptions for secured plugins, which it seems like application/pdf might be in Chrome?

@mikewest
Copy link
Member

I agree with @annevk that this isn't a spec issue; the spec clearly allows user agents to carve out plugins that act equivalently to web content with regard to sandboxing flags. I'm fairly certain that Chrome's PDF plugin does not respect things like allow-scripts and allow-top-navigation (but it's hard to test, given that it doesn't load at all at the moment :) ).

If we can secure it in this way, great! If not, then it seems correct to continue blocking it.

@annevk
Copy link
Member

annevk commented Jan 6, 2021

I think my preference at this point would be that we standardize upon not allowing sandboxed PDF and also start acknowledging PDF as a special type we'll have to account for going forward in page load.

@annevk
Copy link
Member

annevk commented Jan 9, 2021

Note that we have a test for this as well apparently: html/semantics/embedded-content/the-iframe-element/sandbox_004-manual.htm. (I think we could automate that with reftests and possibly fallback for the object element involved.)

domenic added a commit that referenced this issue Aug 9, 2021
Instead, sandboxed iframes are just never allowed to display plugins. (Which, in the modern world, just means PDFs.)

Closes #3958. Helps with #6003.
annevk pushed a commit that referenced this issue Aug 30, 2021
Instead, sandboxed documents are just never allowed to display plugins. (Which, in the modern world, just means PDFs.)

Closes #3958. Helps with #6003.
@kelunik
Copy link

kelunik commented Sep 14, 2021

What's the recommended way to deliver untrusted PDFs if sandboxing isn't one of them?

@domenic
Copy link
Member

domenic commented Sep 14, 2021

I think the recommendation is: don't, since browsers are not capable of defending against malicious PDFs right now.

("Malicious" here = wants to do things disallowed by sandboxing, like navigate the parent page. Browsers can and do try to defend against PDFs that try to do system exploits like arbitrary code execution, as part of their same general code that defends against that for any content.)

@kelunik
Copy link

kelunik commented Sep 14, 2021

Given the protections against arbitrary code execution, I have more faith in browsers handling malicious PDFs than I have in users downloading and opening such files in their average PDF viewer app.

While browsers might currently not support the entire sandboxing set for PDFs, even a subset of the protections would be great, no?

@domenic
Copy link
Member

domenic commented Sep 14, 2021

I agree it would be great! Nobody has implemented it though.

@anforowicz
Copy link
Author

What's the recommended way to deliver untrusted PDFs if sandboxing isn't one of them?

You can consider delivering untrustworthy PDFs from a separate origin or site (e.g. from https://untrustworthy-pdfs.example.com rather than from https://main-site.example.com). This should at least theoretically allow browsers to host such PDFs in a separate process (i.e. introducing a security barrier from the main site).

I recognize that unlike sandboxed iframes this approach doesn't put each PDF in a separate origin. In particular it still allows mixing of PDFs from various sources (e.g. PDFs provided by two different site users).

/CC @kmoon-chromium and @csreis as FYI

mfreed7 pushed a commit to mfreed7/html that referenced this issue Jun 3, 2022
Instead, sandboxed documents are just never allowed to display plugins. (Which, in the modern world, just means PDFs.)

Closes whatwg#3958. Helps with whatwg#6003.
aprotyas added a commit to aprotyas/WebKit that referenced this issue Dec 18, 2024
https://bugs.webkit.org/show_bug.cgi?id=284594
rdar://141166987

Reviewed by NOBODY (OOPS!).

The PDF plugin is an internal WebKit implementation detail, and thus
should not be subjected to the CSP sandbox. This patch makes sure we
bypass the sandbox in SubframeLoader::pluginIsLoadable(). Note that we
only do so for main frame PDFs. The embedded PDF case's behavior is
directed by whatwg/html#3958, and the WPT
`html/semantics/embedded-content/the-iframe-element/sandbox_004.htm`.

Also, add two new API tests that assert correct loading behavior. The
latter enables UnifiedPDFPlugin while the former tests legacy PDF
plugin.

- ContentSecurityPolicy.LoadPDFWithSandboxCSPDirective
- UnifiedPDF.LoadPDFWithSandboxCSPDirective

The rest of the change involves adding new test helpers to facilitate
the API tests, TestWKWebView interface to sample colors (which, by the
way, should be adopted by many tests), and the fallout unified source
build fixes required for a clean build.

* Source/WebCore/html/HTMLPlugInImageElement.h:
* Source/WebCore/loader/SubframeLoader.cpp:
(WebCore::FrameLoader::SubframeLoader::pluginIsLoadable const):
(WebCore::FrameLoader::SubframeLoader::requestPlugin):
(WebCore::FrameLoader::SubframeLoader::pluginIsLoadable): Deleted.
* Source/WebCore/loader/SubframeLoader.h:
* Tools/TestWebKitAPI/SourcesCocoa.txt:
* Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
* Tools/TestWebKitAPI/Tests/WebKit/WKThumbnailView.mm:
* Tools/TestWebKitAPI/Tests/WebKitCocoa/ContentSecurityPolicy.mm:
(TEST(ContentSecurityPolicy, LoadPDFWithSandboxCSPDirective)):
* Tools/TestWebKitAPI/Tests/WebKitCocoa/ContentSecurityPolicyTestHelpers.h: Copied from Tools/TestWebKitAPI/cocoa/CGImagePixelReader.h.
* Tools/TestWebKitAPI/Tests/WebKitCocoa/ContentSecurityPolicyTestHelpers.mm: Copied from Tools/TestWebKitAPI/cocoa/CGImagePixelReader.h.
(TestWebKitAPI::runLoadPDFWithSandboxCSPDirectiveTest):
* Tools/TestWebKitAPI/Tests/WebKitCocoa/CookieAcceptPolicy.mm:

* Tools/TestWebKitAPI/Tests/WebKitCocoa/DocumentEditingContext.mm:
(applyStyle): Deleted.
(applyAhemStyle): Deleted.
* Tools/TestWebKitAPI/Tests/WebKitCocoa/EditorStateTests.mm:
(): Deleted.
(TestWebKitAPI::applyAhemStyle): Deleted.

Fix ODR violation by pushing applyAhemStyle into specific namespaces.

* Tools/TestWebKitAPI/Tests/WebKitCocoa/ElementTargetingTests.mm:
* Tools/TestWebKitAPI/Tests/WebKitCocoa/NavigationAction.mm:
* Tools/TestWebKitAPI/Tests/WebKitCocoa/SiteIsolation.mm:
* Tools/TestWebKitAPI/Tests/WebKitCocoa/TextWidth.mm:
* Tools/TestWebKitAPI/Tests/WebKitCocoa/UnifiedPDFTests.mm:
(TestWebKitAPI::UNIFIED_PDF_TEST):
(TestWebKitAPI::sampleColorsInWebView): Deleted.
* Tools/TestWebKitAPI/Tests/ios/KeyboardInputTestsIOS.mm:
* Tools/TestWebKitAPI/Tests/mac/WKWebViewMacEditingTests.mm:
* Tools/TestWebKitAPI/cocoa/CGImagePixelReader.cpp:
* Tools/TestWebKitAPI/cocoa/CGImagePixelReader.h:
* Tools/TestWebKitAPI/cocoa/HTTPServer.h:
(TestWebKitAPI::HTTPResponse::HTTPResponse):
* Tools/TestWebKitAPI/cocoa/TestWKWebView.h:
* Tools/TestWebKitAPI/cocoa/TestWKWebView.mm:
(-[TestWKWebView sampleColors]):
(-[TestWKWebView sampleColorsWithInterval:]):
aprotyas added a commit to aprotyas/WebKit that referenced this issue Dec 19, 2024
https://bugs.webkit.org/show_bug.cgi?id=284594
rdar://141166987

Reviewed by NOBODY (OOPS!).

The PDF plugin is an internal WebKit implementation detail, and thus
should not be subjected to the CSP sandbox. This patch makes sure we
bypass the sandbox in SubframeLoader::pluginIsLoadable(). Note that we
only do so for main frame PDFs. The embedded PDF case's behavior is
directed by whatwg/html#3958, and the WPT
`html/semantics/embedded-content/the-iframe-element/sandbox_004.htm`.

Also, add two new API tests that assert correct loading behavior. The
latter enables UnifiedPDFPlugin while the former tests legacy PDF
plugin.

- ContentSecurityPolicy.LoadPDFWithSandboxCSPDirective
- UnifiedPDF.LoadPDFWithSandboxCSPDirective

The rest of the change involves adding new test helpers to facilitate
the API tests, TestWKWebView interface to sample colors (which, by the
way, should be adopted by many tests), and the fallout unified source
build fixes required for a clean build.

* Source/WebCore/html/HTMLPlugInImageElement.h:
* Source/WebCore/loader/SubframeLoader.cpp:
(WebCore::FrameLoader::SubframeLoader::pluginIsLoadable const):
(WebCore::FrameLoader::SubframeLoader::requestPlugin):
(WebCore::FrameLoader::SubframeLoader::pluginIsLoadable): Deleted.
* Source/WebCore/loader/SubframeLoader.h:
* Tools/TestWebKitAPI/SourcesCocoa.txt:
* Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
* Tools/TestWebKitAPI/Tests/WebKit/WKThumbnailView.mm:
* Tools/TestWebKitAPI/Tests/WebKitCocoa/ContentSecurityPolicy.mm:
(TEST(ContentSecurityPolicy, LoadPDFWithSandboxCSPDirective)):
* Tools/TestWebKitAPI/Tests/WebKitCocoa/ContentSecurityPolicyTestHelpers.h: Copied from Tools/TestWebKitAPI/cocoa/CGImagePixelReader.h.
* Tools/TestWebKitAPI/Tests/WebKitCocoa/ContentSecurityPolicyTestHelpers.mm: Copied from Tools/TestWebKitAPI/cocoa/CGImagePixelReader.h.
(TestWebKitAPI::runLoadPDFWithSandboxCSPDirectiveTest):
* Tools/TestWebKitAPI/Tests/WebKitCocoa/CookieAcceptPolicy.mm:

* Tools/TestWebKitAPI/Tests/WebKitCocoa/DocumentEditingContext.mm:
(applyStyle): Deleted.
(applyAhemStyle): Deleted.
* Tools/TestWebKitAPI/Tests/WebKitCocoa/EditorStateTests.mm:
(): Deleted.
(TestWebKitAPI::applyAhemStyle): Deleted.

Fix ODR violation by pushing applyAhemStyle into specific namespaces.

* Tools/TestWebKitAPI/Tests/WebKitCocoa/ElementTargetingTests.mm:
* Tools/TestWebKitAPI/Tests/WebKitCocoa/NavigationAction.mm:
* Tools/TestWebKitAPI/Tests/WebKitCocoa/SiteIsolation.mm:
* Tools/TestWebKitAPI/Tests/WebKitCocoa/TextWidth.mm:
* Tools/TestWebKitAPI/Tests/WebKitCocoa/UnifiedPDFTests.mm:
(TestWebKitAPI::UNIFIED_PDF_TEST):
(TestWebKitAPI::sampleColorsInWebView): Deleted.
* Tools/TestWebKitAPI/Tests/ios/KeyboardInputTestsIOS.mm:
* Tools/TestWebKitAPI/Tests/mac/WKWebViewMacEditingTests.mm:
* Tools/TestWebKitAPI/cocoa/CGImagePixelReader.cpp:
* Tools/TestWebKitAPI/cocoa/CGImagePixelReader.h:
* Tools/TestWebKitAPI/cocoa/HTTPServer.h:
(TestWebKitAPI::HTTPResponse::HTTPResponse):
* Tools/TestWebKitAPI/cocoa/TestWKWebView.h:
* Tools/TestWebKitAPI/cocoa/TestWKWebView.mm:
(-[TestWKWebView sampleColors]):
(-[TestWKWebView sampleColorsWithInterval:]):
webkit-commit-queue pushed a commit to aprotyas/WebKit that referenced this issue Dec 19, 2024
https://bugs.webkit.org/show_bug.cgi?id=284594
rdar://141166987

Reviewed by Wenson Hsieh.

The PDF plugin is an internal WebKit implementation detail, and thus
should not be subjected to the CSP sandbox. This patch makes sure we
bypass the sandbox in SubframeLoader::pluginIsLoadable(). Note that we
only do so for main frame PDFs. The embedded PDF case's behavior is
directed by whatwg/html#3958, and the WPT
`html/semantics/embedded-content/the-iframe-element/sandbox_004.htm`.

Also, add two new API tests that assert correct loading behavior. The
latter enables UnifiedPDFPlugin while the former tests legacy PDF
plugin.

- ContentSecurityPolicy.LoadPDFWithSandboxCSPDirective
- UnifiedPDF.LoadPDFWithSandboxCSPDirective

The rest of the change involves adding new test helpers to facilitate
the API tests, TestWKWebView interface to sample colors (which, by the
way, should be adopted by many tests), and the fallout unified source
build fixes required for a clean build.

* Source/WebCore/html/HTMLPlugInImageElement.h:
* Source/WebCore/loader/SubframeLoader.cpp:
(WebCore::FrameLoader::SubframeLoader::pluginIsLoadable const):
(WebCore::FrameLoader::SubframeLoader::requestPlugin):
(WebCore::FrameLoader::SubframeLoader::pluginIsLoadable): Deleted.
* Source/WebCore/loader/SubframeLoader.h:
* Tools/TestWebKitAPI/SourcesCocoa.txt:
* Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
* Tools/TestWebKitAPI/Tests/WebKit/WKThumbnailView.mm:
* Tools/TestWebKitAPI/Tests/WebKitCocoa/ContentSecurityPolicy.mm:
(TEST(ContentSecurityPolicy, LoadPDFWithSandboxCSPDirective)):
* Tools/TestWebKitAPI/Tests/WebKitCocoa/ContentSecurityPolicyTestHelpers.h: Copied from Tools/TestWebKitAPI/cocoa/CGImagePixelReader.h.
* Tools/TestWebKitAPI/Tests/WebKitCocoa/ContentSecurityPolicyTestHelpers.mm: Copied from Tools/TestWebKitAPI/cocoa/CGImagePixelReader.h.
(TestWebKitAPI::runLoadPDFWithSandboxCSPDirectiveTest):
* Tools/TestWebKitAPI/Tests/WebKitCocoa/CookieAcceptPolicy.mm:

* Tools/TestWebKitAPI/Tests/WebKitCocoa/DocumentEditingContext.mm:
(applyStyle): Deleted.
(applyAhemStyle): Deleted.
* Tools/TestWebKitAPI/Tests/WebKitCocoa/EditorStateTests.mm:
(): Deleted.
(TestWebKitAPI::applyAhemStyle): Deleted.

Fix ODR violation by pushing applyAhemStyle into specific namespaces.

* Tools/TestWebKitAPI/Tests/WebKitCocoa/ElementTargetingTests.mm:
* Tools/TestWebKitAPI/Tests/WebKitCocoa/NavigationAction.mm:
* Tools/TestWebKitAPI/Tests/WebKitCocoa/SiteIsolation.mm:
* Tools/TestWebKitAPI/Tests/WebKitCocoa/TextWidth.mm:
* Tools/TestWebKitAPI/Tests/WebKitCocoa/UnifiedPDFTests.mm:
(TestWebKitAPI::UNIFIED_PDF_TEST):
(TestWebKitAPI::sampleColorsInWebView): Deleted.
* Tools/TestWebKitAPI/Tests/ios/KeyboardInputTestsIOS.mm:
* Tools/TestWebKitAPI/Tests/mac/WKWebViewMacEditingTests.mm:
* Tools/TestWebKitAPI/cocoa/CGImagePixelReader.cpp:
* Tools/TestWebKitAPI/cocoa/CGImagePixelReader.h:
* Tools/TestWebKitAPI/cocoa/HTTPServer.h:
(TestWebKitAPI::HTTPResponse::HTTPResponse):
* Tools/TestWebKitAPI/cocoa/TestWKWebView.h:
* Tools/TestWebKitAPI/cocoa/TestWKWebView.mm:
(-[TestWKWebView sampleColors]):
(-[TestWKWebView sampleColorsWithInterval:]):

Canonical link: https://commits.webkit.org/288059@main
webkit-commit-queue pushed a commit to aprotyas/WebKit that referenced this issue Dec 19, 2024
https://bugs.webkit.org/show_bug.cgi?id=284594
rdar://141166987

Reviewed by Wenson Hsieh.

The PDF plugin is an internal WebKit implementation detail, and thus
should not be subjected to the CSP sandbox. This patch makes sure we
bypass the sandbox in SubframeLoader::pluginIsLoadable(). Note that we
only do so for main frame PDFs. The embedded PDF case's behavior is
directed by whatwg/html#3958, and the WPT
`html/semantics/embedded-content/the-iframe-element/sandbox_004.htm`.

Also, add two new API tests that assert correct loading behavior. The
latter enables UnifiedPDFPlugin while the former tests legacy PDF
plugin.

- ContentSecurityPolicy.LoadPDFWithSandboxCSPDirective
- UnifiedPDF.LoadPDFWithSandboxCSPDirective

The rest of the change involves adding new test helpers to facilitate
the API tests, TestWKWebView interface to sample colors (which, by the
way, should be adopted by many tests), and the fallout unified source
build fixes required for a clean build.

* Source/WebCore/html/HTMLPlugInImageElement.h:
* Source/WebCore/loader/SubframeLoader.cpp:
(WebCore::FrameLoader::SubframeLoader::pluginIsLoadable const):
(WebCore::FrameLoader::SubframeLoader::requestPlugin):
(WebCore::FrameLoader::SubframeLoader::pluginIsLoadable): Deleted.
* Source/WebCore/loader/SubframeLoader.h:
* Tools/TestWebKitAPI/SourcesCocoa.txt:
* Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
* Tools/TestWebKitAPI/Tests/WebKit/WKThumbnailView.mm:
* Tools/TestWebKitAPI/Tests/WebKitCocoa/ContentSecurityPolicy.mm:
(TEST(ContentSecurityPolicy, LoadPDFWithSandboxCSPDirective)):
* Tools/TestWebKitAPI/Tests/WebKitCocoa/ContentSecurityPolicyTestHelpers.h: Copied from Tools/TestWebKitAPI/cocoa/CGImagePixelReader.h.
* Tools/TestWebKitAPI/Tests/WebKitCocoa/ContentSecurityPolicyTestHelpers.mm: Copied from Tools/TestWebKitAPI/cocoa/CGImagePixelReader.h.
(TestWebKitAPI::runLoadPDFWithSandboxCSPDirectiveTest):
* Tools/TestWebKitAPI/Tests/WebKitCocoa/CookieAcceptPolicy.mm:

* Tools/TestWebKitAPI/Tests/WebKitCocoa/DocumentEditingContext.mm:
(applyStyle): Deleted.
(applyAhemStyle): Deleted.
* Tools/TestWebKitAPI/Tests/WebKitCocoa/EditorStateTests.mm:
(): Deleted.
(TestWebKitAPI::applyAhemStyle): Deleted.

Fix ODR violation by pushing applyAhemStyle into specific namespaces.

* Tools/TestWebKitAPI/Tests/WebKitCocoa/ElementTargetingTests.mm:
* Tools/TestWebKitAPI/Tests/WebKitCocoa/NavigationAction.mm:
* Tools/TestWebKitAPI/Tests/WebKitCocoa/SiteIsolation.mm:
* Tools/TestWebKitAPI/Tests/WebKitCocoa/TextWidth.mm:
* Tools/TestWebKitAPI/Tests/WebKitCocoa/UnifiedPDFTests.mm:
(TestWebKitAPI::UNIFIED_PDF_TEST):
(TestWebKitAPI::sampleColorsInWebView): Deleted.
* Tools/TestWebKitAPI/Tests/ios/KeyboardInputTestsIOS.mm:
* Tools/TestWebKitAPI/Tests/mac/WKWebViewMacEditingTests.mm:
* Tools/TestWebKitAPI/cocoa/CGImagePixelReader.cpp:
* Tools/TestWebKitAPI/cocoa/CGImagePixelReader.h:
* Tools/TestWebKitAPI/cocoa/HTTPServer.h:
(TestWebKitAPI::HTTPResponse::HTTPResponse):
* Tools/TestWebKitAPI/cocoa/TestWKWebView.h:
* Tools/TestWebKitAPI/cocoa/TestWKWebView.mm:
(-[TestWKWebView sampleColors]):
(-[TestWKWebView sampleColorsWithInterval:]):

Canonical link: https://commits.webkit.org/288060@main
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
interop Implementations are not interoperable with each other topic: embed and object topic: sandbox
Development

Successfully merging a pull request may close this issue.

5 participants