Might be nice to remove the PRIVATE permission; I know this is used to show modal dialogs via JS, but not sure if there are any other uses.

https://bugs.chromium.org/p/chromium/issues/detail?id=54748 gave us a solution for passwords, perphaps this could be expanded to cover alert/prompt cases?

dsinclair: Would you mind helping us find an owner for this?

mbarbella: All of the fixes to close these holes will be on the extensions side, outside of PDFium.  Adding extensions component.

> There's a renderer-side
check for filesystem: URLs, so the attack page has to use the v8 bug again to bypass it.

I'm not sure what this means. What exactly is problematic about opening a filesystem URL?

One clear thing to fix seems to be that the PDF plugin shouldn't be allowed to Execute Script as it doesn't need to (I believe) and certainly not in chrome:// pages.

It's not clear whether the PDFium extension needs the file IO API, either. 

The footnotes from the original are:

1 - https://cs.chromium.org/chromium/src/v8/src/builtins/builtins-array-gen.cc?rcl=b2a75aa261671bca28890a5dfbce87d96d8eaef4&l=1945
2 - https://cs.chromium.org/chromium/src/content/browser/child_process_security_policy_impl.cc?rcl=61eb267d400f1b4816e0d91e0a4bd86bd417b26d&l=627
3 - https://cs.chromium.org/chromium/src/chrome/browser/browser_process_impl.cc?rcl=8d0e11cc9ffb098fd0f4405b95807cc67965fd64&l=1073
4 - https://cs.chromium.org/chromium/src/chrome/common/chrome_content_client.cc?rcl=3c96c6db257673bd838d2c1f30ee89e70e5fe708&l=117
5 - https://cs.chromium.org/chromium/src/chrome/browser/ui/webui/md_downloads/md_downloads_dom_handler.cc?rcl=ad6860b020ad4ecfa42c78cc31217954bc24fe2a&l=211

+avi re: comment 1.

Re #5: the PPB_FileIO API is exposed to all plugins. I don't think the fact that the plugin can write to a file is problematic in itself.

I'm still struggling a bit to understand what the action items are here. 
-The V8 bug described is covered in  issue 821137 .
-It seems like there's something problematic about opening the filesystem:chrome-extension: URL but I don't understand it. +creis and dcheng who may understand more. This should be filed as a separate issue, if it is indeed an issue.
-We should restrict the PDF plugin from executing script as it's not needed. However judging by the OP this is just a simplification to the exploit. We should file a separate bug for this. 
-There seems like there is a bug in the download manager which is triggerable through an extensions API (based on the last paragraph). Is this covered by another bug or do we need to file one?

Assigning back to mbarbella for tracking based on #8. As far as I can tell the only PDF/plugin related part is in  issue 821266 . 

Re comment 1:

+tsepez, you have fundamental misunderstandings.

You point to a CL that implemented a dialog for the password, and ask if we can extend it for other dialogs. That CL was implemented as a stopgap because the PDF plugin was being rewritten to allow doing UI itself. The code in that CL no longer exists in Chrome, as the plugin now handles dialogs itself.

If you are asking if the plugin's support for password dialogs can be extended to handle other dialogs, perhaps. I would rather support removal of JavaScript dialogs from PDF wholesale. I see their use as entirely consisting of abuse.

Re comment 8:

"We should restrict the PDF plugin from executing script as it's not needed." That sounds like a policy decision, to drop all JavaScript support. I would support that, too.

I don't think dropping JS support from PDFs is feasible. JS is used for a lot of things in PDF forms, calculations, validation, submitting forms, etc. Removing this support would just break all that functionality.

Or do you mean dropping JS from something else?

For dialogs in PDFs, they can also be used to display validation warnings (which we've seen in a few PDF files)

Then I'm not sure what the suggestion in the second sentence in comment 1 was. tsepez, can you clarify?

I was under the impression that app.alert() in a PDF as evaluated by V8 in the ppapi process eventually results in a window.alert() call as evaluated by V8 in the extension process, and whether the work you had done previously could be extended so as to prevent the need to execute JS in the extension process in response to any incoming request from the PPAPI.

That's an interesting question. I am not up-to-speed on the implementation details of app.alert().

In other words, JS is executing in two places - in the PPAPI process as part of PDFium, and in the extension process to do the controls and such. 

https://cs.chromium.org/chromium/src/ppapi/c/private/ppb_instance_private.h?rcl=0117469878380cefe143071e231e239d806ac048&l=60 provides a bridge between the two.

I'd like to deny PDF's ppapi process the right to use that interface. A compomised ppapi can make the executescript call.

But taking away that right breaks

https://cs.chromium.org/chromium/src/pdf/out_of_process_instance.cc?rcl=0117469878380cefe143071e231e239d806ac048&l=338

and so I was looking for a workaround, but as you say, the password stuff is no longer there.

I gotcha.

The TODO says we can switch to "a custom PPB_PDF function." If you have someone to wire that up to the chrome/browser layer I'll totally wire it from there. If not I could work it out, but it's been a while since I've dug around in this code and it's changed a bit.

Avi, as it turns out, I don't think this will have to come up high enough in the stack to where we need custom UI. The problem is really that we're handing back a proxy to the window object -- using window.alert() would be OK if there were a path that did *only* that.  Thanks for your help as always.

Nice report!  I'm talking with alexmos@, nick@, dcheng@, and lukasza@ about multiple places where things are going wrong and what defenses we can add.  I'll write up more of what we find shortly, but just wanted to post that we're actively looking at it at several levels.

A few areas we're looking at so far:

1) Why does the PDFium extension process have script access to chrome:// URLs, and can we prevent that?  (Both versions of the attack use this.)

2) Why does the PDF plugin have a private API for running script in the PDFium extension process, and can we restrict that?  (The second version of the attack uses this.)

3) Why are we relying on a renderer side check (for user gestures) to prevent dangerous downloads, and can we do something on the browser side?  Daniel has a proposal here that he started just yesterday.  (Both versions of the attack use this.)

4) Why isn't the web process prevented from opening filesystem:chrome-extension:// URLs?  That's a local URL to an origin the process can't have pages from, so the browser process should be able stop that.  Nick and Alex are looking into this.  (This is used in the first version of the attack.)

5) Why does the PDFium extension declare everything to be web accessible in its manifest?  Alex just spotted this and it probably explains why Web Accessible Resources (WAR) checks don't prevent this.  Long term, can we prevent wildcards in the manifest for WAR?  (This is used in the first version of the attack.)

6) Why is the PPAPI plugin process able to create file URLs in the extension's origin?  (This is used in the first version of the attack.)


I know some of these are referenced above as well.  We'll coordinate and come up with answers and responses with tsepez@, raymes@, etc.  Also CC'ing ekaramad for additional PDFium knowledge, Devlin for extension manifest restrictions, and asanka@ for downloads knowledge.

Also +weili for PDFium

+dbeam as FYI for WebUI and MD Downloads.

+dtrainor for general downloads internals.

Just adding a (maybe unrelated) quick side note that we also <embed> the PDF plugin for print preview. The preview preview page is simply the index.html of the component extension itself which is loaded inside an <iframe> inside chrome://print (WebUI?). The actual PDF source however should be safe given that chrome generates it itself.

Removing myself as owner so it's clear that this is available. If anyone from the cc list is planning on working on a fix, please assign yourself.

I'll take ownership to ensure that fixes land and that we keep making progress, whether or not I'm the one to land the CLs.

Thanks for picking this up creis. 
I'll try to answer a few from #24:
> 1) Why does the PDFium extension process have script access to chrome:// URLs, and can we prevent that?  (Both versions of the attack use this.)

Currently print preview embeds the plugin, which is a chrome://print URL, but that's separate from the extension. I don't know the answer to why the extension has access to chrome:// URLs directly.

> 2) Why does the PDF plugin have a private API for running script in the PDFium extension process, and can we restrict that?  (The second version of the attack uses this.)

This is mostly result of the PDF plugin being a private plugin which gets access to a bunch more APIs (basically almost the same ones as flash). We're looking into locking this down more in  issue 821266 .

> 5) Why does the PDFium extension declare everything to be web accessible in its manifest?  

It turns out this was done so that chrome://print can include the same resources as the extension. I'm sure this can be achieved another way. I'll file a separate issue to investigate doing that.

> 6) Why is the PPAPI plugin process able to create file URLs in the extension's origin?  (This is used in the first version of the attack.)

I believe plugins in general will create files in whatever origin they are embedded in. I can verify this. Does this seem problematic in general? 

Thanks, raymes@!  A few other updates on the topics from comment 24, plus questions for specific owners below.  Note that it's important that we land enough fixes to disrupt both attacks.

> 1) Why does the PDFium extension process have script access to chrome:// URLs, and can we prevent that?  (Both versions of the attack use this.)

thestig@ and dcheng@ are discussing options here.  This seems like the most desirable place to disrupt the attack, since it's privilege escalation to WebUI and is central to both versions of the attack.  For context, PDFium needs to be able to request chrome://print/$ID/$PAGENUM.pdf for print preview, but having that in the host permission of the manifest means it *also* has the ability run script on chrome:// URLs.  Scripting is likely possible using either chrome.tabs.update to a javascript: URL (as in the report) or via chrome.tabs.executeScript.  We don't think that scripting access should be needed for PDFium's functionality, but it's not clear if there's a way specify request access without script access in extension manifests.

dcheng@ is doing some testing of this in code.  One option might be limiting the host permission to chrome://print/* rather than all of chrome://* (or using <all_urls>, like it does now in the manifest).  That would still allow script access to chrome://print URLs, though.  That's fairly dangerous in its own right.

I've filed  issue 821613  as a separate blocking bug to discuss our options.
 

> 2) Why does the PDF plugin have a private API for running script in the PDFium extension process, and can we restrict that?  (The second version of the attack uses this.)

Thanks for the pointer to  issue 821266 !  Agreed that we should lock it down in that bug.


> 3) Why are we relying on a renderer side check (for user gestures) to prevent dangerous downloads, and can we do something on the browser side?  (Both versions of the attack use this.)

This is related to issue 595841, which we haven't been able to get traction on.  Opening a dangerous download from chrome://downloads requires a user gesture check, but this can be bypassed in the renderer process.  Daniel has a public proposal (from yesterday) to do the check in the browser process instead:
https://docs.google.com/document/d/1SuuaGe-d64FEz0ZMGu5FAEEJtMSvL77YGz4V9J1NUwo/edit

If we can't do that quickly, I'd like us to seriously consider a browser-process prompt in chrome://downloads specific to opening dangerous downloads.  asanka@ and dcheng@ should compare notes on these approaches, since we can't keep leaving this hole open.


> 4) Why isn't the web process prevented from opening filesystem:chrome-extension:// URLs?  That's a local URL to an origin the process can't have pages from, so the browser process should be able stop that.  (This is used in the first version of the attack.)

Nick filed tracking bugs for this in  issue 821596  and  issue 821586 , and he has fixes in progress at https://chromium-review.googlesource.com/c/chromium/src/+/961126.  Basically, the browser process should be enforcing that local schemes can only be requested from a process if it might have been able to commit a URL in that process.  Isolate-extensions means that chrome-extension:// URLs can't commit in normal web renderers, so we can block a web page from opening a filesystem:chrome-extension:// URL.



> 5) Why does the PDFium extension declare everything to be web accessible in its manifest?  Alex just spotted this and it probably explains why Web Accessible Resources (WAR) checks don't prevent this.  Long term, can we prevent wildcards in the manifest for WAR?  (This is used in the first version of the attack.)

raymes@: Thanks-- please link the bug for this once you file it.


> 6) Why is the PPAPI plugin process able to create file URLs in the extension's origin?  (This is used in the first version of the attack.)

raymes@: If it's core to how plugins work, that may be ok.
tsepez@/thestig@: Can we remove the file IO API permission from the PDFium PPAPI process, per comment 5?

Thanks creis!

> tsepez@/thestig@: Can we remove the file IO API permission from the PDFium PPAPI process, per comment 5?

The FileIO API is a basic permission that all plugins get access to (e.g. all NaCl plugins as well). It shouldn't be very much more powerful than the file APIs exposed on the web platform. 

raymes: to clarify, the concern about web_accessible_resources is that wildcard rules might allow filesystem: URLs created by in a chrome-extension:// origin to become web accessible. But I'm not sure if this is still an issue if  issue 821586  is fixed: it seems like the web accessible resource checks are skipped in some cases (!!)

Quick update: dcheng@ landed r543030 to prevent the PDFium extension process from being able to script chrome:// URLs ( issue 821613 ), which should disrupt both versions of the attack.  (This is task (1) from comment 24.)  Thanks dcheng@!  It should be possible to verify on ToT or tomorrow's canary

We'll continue work on the other followup bugs.

mbarbella@: Should we wait for more of the sub-bugs to be fixed or mark this (or issue 820838) fixed now?

Status updates from today:

> 1) Why does the PDFium extension process have script access to chrome:// URLs, and can we prevent that?  (Both versions of the attack use this.)

This is fixed by r543030 in  issue 821613 .  PDFium now only has chrome://resources in its host permissions, so it can't script other arbitrary chrome:// URLs.

dcheng@: Can you file a followup issue for the ServiceWorker concern you had?  I agree it's worth ensuring that an attacker can't put a ServiceWorker on chrome://resources.

rdevlin.cronin@: Can you file a followup issue for excluding chrome:// URLs from <all_urls> in component extensions, so that they have to be explicitly requested if desired?  That seemed like a nice way to avoid unintentional bugs in the future. 


> 2) Why does the PDF plugin have a private API for running script in the PDFium extension process, and can we restrict that?  (The second version of the attack uses this.)

Ongoing discussion of this in  issue 821266 .  Hopefully we can restrict that access.


> 3) Why are we relying on a renderer side check (for user gestures) to prevent dangerous downloads, and can we do something on the browser side?  (Both versions of the attack use this.)

dcheng@: Can you comment on whether your user gesture proposal can get traction soon?  (I wonder if we can find someone to help build it, to avoid cutting into your time.)  :)


> 4) Why isn't the web process prevented from opening filesystem:chrome-extension:// URLs?  That's a local URL to an origin the process can't have pages from, so the browser process should be able stop that.  (This is used in the first version of the attack.)

This is fixed by r543142 in  issue 821596 .


> 5) Why does the PDFium extension declare everything to be web accessible in its manifest?  Alex just spotted this and it probably explains why Web Accessible Resources (WAR) checks don't prevent this.  Long term, can we prevent wildcards in the manifest for WAR?  (This is used in the first version of the attack.)

raymes@ has a CL in progress in issue 821652.  We've determined it probably wouldn't have stopped the attack in the first place, but it's good for cleanup and least privilege.


> 6) Why is the PPAPI plugin process able to create file URLs in the extension's origin?  (This is used in the first version of the attack.)

Seems like this is probably unimportant.

+awhalley@ (Security TPM)
+abdulsyed@ & cmasso@ & josafat@ (M66 TPMs)

Status updates, pruned to the issues that still have work remaining:

> 1) Why does the PDFium extension process have script access to chrome:// URLs, and can we prevent that?  (Both versions of the attack use this.)

Still debating whether the merge to M66 is safe.  Hopefully so and we can proceed before the next beta is cut.

I filed issue 823475 for dcheng@'s ServiceWorker concern.  Devlin filed  issue 821858  for not auto-granting chrome:// to component extensions with <all_urls>.


> 2) Why does the PDF plugin have a private API for running script in the PDFium extension process, and can we restrict that?  (The second version of the attack uses this.)

Sounds like this will take multiple CLs in  issue 821266  and might not be mergeable, but still aiming for M67.  There's followup audit work in issue 823171.


> 3) Why are we relying on a renderer side check (for user gestures) to prevent dangerous downloads, and can we do something on the browser side?  (Both versions of the attack use this.)

I'll discuss with dcheng@ when he gets back.  Still hoping we can get browser-side enforcement for user gestures here.


> 4) Why isn't the web process prevented from opening filesystem:chrome-extension:// URLs?  That's a local URL to an origin the process can't have pages from, so the browser process should be able stop that.  (This is used in the first version of the attack.)

The fix for this was merged to M66 in  issue 821596 , so this one is done.


> 5) Why does the PDFium extension declare everything to be web accessible in its manifest?  Alex just spotted this and it probably explains why Web Accessible Resources (WAR) checks don't prevent this.  Long term, can we prevent wildcards in the manifest for WAR?  (This is used in the first version of the attack.)

raymes@'s CL in issue 821652 is almost ready to land.

After chatting with mbarbella@, I'm going to mark this as fixed, though we're still tracking the ExecuteScript mitigation in  issue 821266  and downloads work.  Both versions of the privilege escalation have been disrupted in  issue 821613  by preventing script access to chrome://, and the local URL check was fixed in issue  issue 821596 .  This means the attack shouldn't be possible anymore, even with a different renderer compromise.  I'll continue to post updates on the progress of the remaining issues.

Status updates on the remaining issues:

> 1) Why does the PDFium extension process have script access to chrome:// URLs, and can we prevent that?  (Both versions of the attack use this.)

The fix has been merged to M66 and should go out in tomorrow's beta.  dcheng@ has requested merge to M65 as well, just in case there ends up being a respin to include it.

ServiceWorkers should already be prevented on chrome:// URLs, but falken@ will add verification in issue 823475.  dbertoni@ will handle the <all_urls> thing in  issue 821858 .

I think we still want some confirmation that there aren't other component extensions with this problem.  Devlin, is there a way to audit them?


> 2) Why does the PDF plugin have a private API for running script in the PDFium extension process, and can we restrict that?  (The second version of the attack uses this.)

Still waiting on  issue 821266  and the audit work in issue 823171.


> 3) Why are we relying on a renderer side check (for user gestures) to prevent dangerous downloads, and can we do something on the browser side?  (Both versions of the attack use this.)

We're discussing browser-side enforcement of user gesture in  issue 823864 .  Seems to be plausible with chrome://flags/#user-activation-v2.


> 5) Why does the PDFium extension declare everything to be web accessible in its manifest?  Alex just spotted this and it probably explains why Web Accessible Resources (WAR) checks don't prevent this.  Long term, can we prevent wildcards in the manifest for WAR?  (This is used in the first version of the attack.)

The fix for this landed in r544484 in issue 821652, so this one is done.

> I think we still want some confirmation that there aren't other component extensions with this problem.  Devlin, is there a way to audit them?

No streamlined way - only to look at all extensions that are added by the ComponentLoader in extensions code.

dcheng@ tried to audit these in https://bugs.chromium.org/p/chromium/issues/detail?id=821613#c4, and found no other component extensions relying on this.

This bug requires manual review: M66 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), josafat@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

The change  issue 821266  to change how we show the modal dialog in the PDF extension has been committed. The bug is still open as there is a follow-on CL to change the PDF permissions.

I don't think there is anything to merge here (sherrifbot auto merge-request label). Creis@ can you please confirm?

r.e. #33 and bug 595841 - We are making the change (native dialog in chrome://downloads and extension downloads API) and are working with UX right now on the exact string.

Comment 52: Correct, no merge needed here.

Status updates on the remaining issues:

> 1) Why does the PDFium extension process have script access to chrome:// URLs, and can we prevent that?  (Both versions of the attack use this.)

The fix from  issue 821613  has been merged to M65 as well, in case there's a stable respin.

I've sent pings to issue 823475 and  issue 821858  for further hardening work.  As Devlin notes, the audit of other component extensions was basically done in  https://crbug.com/821613#c4 .

This one is basically done apart from the followup hardening work.


> 2) Why does the PDF plugin have a private API for running script in the PDFium extension process, and can we restrict that?  (The second version of the attack uses this.)

ExecuteScript was revoked from the PDF plugin process in r546345 for  issue 821266 .

For the audit in issue 823171, some additional restrictions were added in r545916, and it sounds like more might be on the way (?).


> 3) Why are we relying on a renderer side check (for user gestures) to prevent dangerous downloads, and can we do something on the browser side?  (Both versions of the attack use this.)

A browser-side enforcement of user gesture for WebUI pages landed in r547114, but it got reverted in r547126.  dcheng@ is trying to update it to land again.

Also, a dangerous download prompt was added in r547094 for issue 595841.


> 4) Why isn't the web process prevented from opening filesystem:chrome-extension:// URLs?  That's a local URL to an origin the process can't have pages from, so the browser process should be able stop that.  (This is used in the first version of the attack.)

The basic fix for this from  issue 821596  (r543142) caused a regression in  issue 825111 .  nick@ landed r547218 instead as an alternate fix (blocking the navigation in ShouldAllowOpenURL), and he's reverting the original fix in https://chromium-review.googlesource.com/c/chromium/src/+/982645 to fix the regression.

This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot