Issue metadata
Sign in to add a comment
|
Restrict PDFium extension from running script inside chrome:// URLs |
||||||||||||||||||||||
Issue descriptionAs reported in issue 821138 (under issue 820838), the PDFium extension has the ability to run script inside chrome:// URLs. This is due to the <all_urls> permission in its manifest (and possibly the fact it's a component extension): https://cs.chromium.org/chromium/src/chrome/browser/resources/pdf/manifest.json As a result, the extension process can use chrome.tabs.update with a JavaScript URL or chrome.tabs.executeScript to run script inside of a WebUI page, which can be used as a step in a sandbox escape.
,
Mar 14 2018
,
Mar 14 2018
> (and possibly the fact it's a component extension) Definitely :) We certainly don't allow non-component extensions to run script on chrome:// pages. It definitely seems like we should be able to restrict this significantly to specify appropriate hosts. In issue 821138 , you mentioned that the extension needs to fetch resources from chrome://print - is that the case? If so, how did it work when we just restricted to chrome://resources? Looking farther ahead, it would be nice to not auto-grant chrome:// pages to component extensions, even if they request <all_urls>. Unfortunately, this might be a breaking change, and would involve auditing (and possibly updating) existing extensions. Still absolutely worth doing, but not something we can do as an immediate fix (unlike, perhaps, updating the pdf extension).
,
Mar 14 2018
#3: I think the way that the PDFium plugin is passed the pdf resource has changed to no longer depend on host permissions for this. I audited all the other (hopefully) component extension manifests. No other extension appears to specify <all_urls>; the only things I saw were chrome://theme and chrome://resources. So maybe we can already do that change?
,
Mar 14 2018
sammc and I checked a few other things and the fix seems good. Namely we were a bit worried about network mounts and range requests but they work fine. In the process we discovered that we should lock down the plugin to only be able to make requests on the same origin as the PDF that it's loading (currently it can make cross origin requests, as can flash). But I'll file a separate bug for this.
,
Mar 14 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d8206c55039dc71555ea92bde3080c8743ad11b8 commit d8206c55039dc71555ea92bde3080c8743ad11b8 Author: Daniel Cheng <dcheng@chromium.org> Date: Wed Mar 14 08:40:45 2018 Only grant host permissions for chrome://resources/* to PDF component extension Bug: 821613 Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation Change-Id: Icdaab0eb77a72a7b6ad0c491424672614c24a9fa Reviewed-on: https://chromium-review.googlesource.com/961727 Reviewed-by: Raymes Khoury <raymes@chromium.org> Reviewed-by: Sam McNally <sammc@chromium.org> Reviewed-by: Lei Zhang <thestig@chromium.org> Commit-Queue: Daniel Cheng <dcheng@chromium.org> Cr-Commit-Position: refs/heads/master@{#543030} [modify] https://crrev.com/d8206c55039dc71555ea92bde3080c8743ad11b8/chrome/browser/resources/pdf/manifest.json
,
Mar 14 2018
,
Mar 14 2018
,
Mar 14 2018
,
Mar 14 2018
,
Mar 14 2018
,
Mar 14 2018
+awhalley@(Security TPM), is this block M65 further roll out?
,
Mar 14 2018
Nope, will not block rollout.
,
Mar 15 2018
OK, Is this need to be included in next week M65 respin? The change is not yet baked in canary/dev/beta.
,
Mar 15 2018
Ideally, yes. I've set a next action to check on the next canary with this CL.
,
Mar 15 2018
The NextAction date has arrived: 2018-03-15
,
Mar 15 2018
dcheng@: Thanks for following up to verify and request merge. One sanity check: was the <all_urls> line necessary in the past, and stopped being necessary at some point? Or was it never necessary? Just want to make sure we're not risking a regression when we merge this to M66 and M65.
,
Mar 15 2018
This won't make a dev or beta before the stable merge is needed, so it doesn't meet the normal bar for a stable merge. We'll pick it up in 66 (or a later 65 spin if any)
,
Mar 15 2018
I tried to figure out what changed to make this no longer require host permissions. In a version of Chrome that predates sammc's fix for opening pdfs on shares (https://chromium.googlesource.com/chromium/src/+/c064ddad5554afe987ef088d4bf1165cc7eda74e), here's the error: Refused to evaluate a string as JavaScript because 'unsafe-eval' is not an allowed source of script in the following Content Security Policy directive: "script-src 'self' chrome://resources". pdf.js:465 Not allowed to load local resource: file://dcheng2-w/Users/dcheng/Downloads/pdftest/f1040.pdfpdf.js:465 PDFViewer.afterZoom_ extensions::uncaught_exception_handler:8 Error in response to tabs.setZoomSettings: TypeError: this.plugin_.postMessage is not a function at Object.PDFViewer.afterZoom_ (chrome-extension://mhjfbmdgcfjbbpaeojofohoefgiehjai/pdf.js:465:18) at Object.PDFViewer (chrome-extension://mhjfbmdgcfjbbpaeojofohoefgiehjai/pdf.js:132:17) at initViewer (chrome-extension://mhjfbmdgcfjbbpaeojofohoefgiehjai/main.js:38:14) at Object.<anonymous> (chrome-extension://mhjfbmdgcfjbbpaeojofohoefgiehjai/main.js:80:7) Examining PDFViewer.plugin_ gives: <embed id="plugin" type="application/x-google-chrome-pdf" src="file://dcheng2-w/Users/dcheng/Downloads/pdftest/f1040.pdf" stream-url="blob:chrome-extension://mhjfbmdgcfjbbpaeojofohoefgiehjai/36C0D1AC-8D53-4202-AD68-373E279EF494" headers="" full-frame="" style="height: 1247px; width: 1249px;"> So the source is directly embedded, which would explain the console error. However, looking a recent build of Chrome and examining PDFViewer.plugin_ *also* gives this as the outerHTML: <embed id="plugin" type="application/x-google-chrome-pdf" src="file://dcheng2-w/Users/dcheng/Downloads/pdftest/f1040.pdf" stream-url="blob:chrome-extension://mhjfbmdgcfjbbpaeojofohoefgiehjai/cdcd072f-acd2-4361-a8aa-edb383be80dc" headers="" background-color="0xFF525659" top-toolbar-height="56" full-frame=""> Which is essentially the same thing but somehow doesn't trigger any errors now. I've also verified that the PDF viewer still works on Canary, which does have my CL... so I have no clue. Maybe we changed some plumbing for mime_handler_view...? One potential CL that seems like it could have affected this? It mentions fixing something for top-level PDFs: maybe embedding PDFs on file:// https://chromium.googlesource.com/chromium/src/+/1d0b49e4794131e5f53265bf85d961020a725a30 Though it doesn't seem like a great candidate. raymes, sammc: any ideas?
,
Mar 15 2018
Not sure if this helps, but here's the stack where we hit the local load failure message in the build from https://commondatastorage.googleapis.com/chromium-browser-continuous/index.html?prefix=Win/313166/: 00 009bdbc4 69a10580 chrome_child!blink::Document::addConsoleMessage [c:\b\build\slave\win\build\src\third_party\webkit\source\core\dom\document.cpp @ 4914] 01 009bdbe4 69931693 chrome_child!blink::FrameLoader::reportLocalLoadFailed+0xd0 [c:\b\build\slave\win\build\src\third_party\webkit\source\core\loader\frameloader.cpp @ 823] 02 009bdc04 699319f3 chrome_child!blink::HTMLPlugInElement::pluginIsLoadable+0x63 [c:\b\build\slave\win\build\src\third_party\webkit\source\core\html\htmlpluginelement.cpp @ 631] 03 009bdc70 69975006 chrome_child!blink::HTMLPlugInElement::requestObject+0xa3 [c:\b\build\slave\win\build\src\third_party\webkit\source\core\html\htmlpluginelement.cpp @ 503] 04 009bdcac 69931f92 chrome_child!blink::HTMLEmbedElement::updateWidgetInternal+0xa6 [c:\b\build\slave\win\build\src\third_party\webkit\source\core\html\htmlembedelement.cpp @ 150] 05 009bdcb4 69a09817 chrome_child!blink::HTMLPlugInElement::updateWidget+0x12 [c:\b\build\slave\win\build\src\third_party\webkit\source\core\html\htmlpluginelement.cpp @ 189] 06 009bdcec 69a027aa chrome_child!blink::FrameView::updateWidgets+0xa7 [c:\b\build\slave\win\build\src\third_party\webkit\source\core\frame\frameview.cpp @ 2015] 07 (Inline) -------- chrome_child!blink::FrameView::updateWidgetsTimerFired+0x18 [c:\b\build\slave\win\build\src\third_party\webkit\source\core\frame\frameview.cpp @ 2031] 08 009bdcf8 698d3571 chrome_child!blink::FrameView::flushAnyPendingPostLayoutTasks+0x4a [c:\b\build\slave\win\build\src\third_party\webkit\source\core\frame\frameview.cpp @ 2042] 09 009bdd0c 699318b0 chrome_child!blink::Document::updateLayoutIgnorePendingStylesheets+0x91 [c:\b\build\slave\win\build\src\third_party\webkit\source\core\dom\document.cpp @ 1960] 0a 009bdd18 699317ce chrome_child!blink::HTMLPlugInElement::renderPartForJSBindings+0x10 [c:\b\build\slave\win\build\src\third_party\webkit\source\core\html\htmlpluginelement.cpp @ 399] 0b (Inline) -------- chrome_child!blink::HTMLPlugInElement::pluginWidgetForJSBindings+0x8 [c:\b\build\slave\win\build\src\third_party\webkit\source\core\html\htmlpluginelement.cpp @ 334] 0c 009bdd2c 69f4cc9c chrome_child!blink::HTMLPlugInElement::pluginWrapper+0x3e [c:\b\build\slave\win\build\src\third_party\webkit\source\core\html\htmlpluginelement.cpp @ 317] 0d 009bdd68 69f4d159 chrome_child!blink::`anonymous namespace'::getScriptableObjectProperty<blink::V8HTMLEmbedElement,v8::Local<v8::String> >+0x2c [c:\b\build\slave\win\build\src\third_party\webkit\source\bindings\core\v8\custom\v8htmlpluginelementcustom.cpp @ 53] 0e 009bdd78 69f023f9 chrome_child!blink::V8HTMLObjectElement::namedPropertyGetterCustom+0x39 [c:\b\build\slave\win\build\src\third_party\webkit\source\bindings\core\v8\custom\v8htmlpluginelementcustom.cpp @ 121] 0f 009bdd88 696d599e chrome_child!blink::HTMLObjectElementV8Internal::namedPropertyGetterCallback+0x19 [c:\b\build\slave\win\build\src\out\release\gen\blink\bindings\core\v8\v8htmlobjectelement.cpp @ 719] 10 009bddb4 695e06a7 chrome_child!v8::internal::PropertyCallbackArguments::Call+0x6e [c:\b\build\slave\win\build\src\v8\src\arguments.cc @ 87] 11 009bde04 695df8c1 chrome_child!v8::internal::JSObject::GetPropertyWithInterceptor+0x127 [c:\b\build\slave\win\build\src\v8\src\objects.cc @ 13555] 12 009bde2c 696db34c chrome_child!v8::internal::Object::GetProperty+0x31 [c:\b\build\slave\win\build\src\v8\src\objects.cc @ 123] 13 009bdebc 696db9a5 chrome_child!v8::internal::LoadIC::Load+0x2fc [c:\b\build\slave\win\build\src\v8\src\ic\ic.cc @ 742] 14 (Inline) -------- chrome_child!v8::internal::__RT_impl_LoadIC_Miss+0x1cb [c:\b\build\slave\win\build\src\v8\src\ic\ic.cc @ 2293] 15 009bdff0 2ea0d4dc chrome_child!v8::internal::LoadIC_Miss+0x1d5 [c:\b\build\slave\win\build\src\v8\src\ic\ic.cc @ 2260] WARNING: Frame IP not in any known module. Following frames may be wrong. 16 009be010 3dac9f58 0x2ea0d4dc 17 009be034 2ea30755 0x3dac9f58 18 009be050 2ea168ca 0x2ea30755 19 009be08c 695fff83 0x2ea168ca 1a 009be0e8 695ff15c chrome_child!v8::internal::Invoke+0x213 [c:\b\build\slave\win\build\src\v8\src\execution.cc @ 129] 1b 009be11c 697b15d9 chrome_child!v8::internal::Execution::Call+0x13c [c:\b\build\slave\win\build\src\v8\src\execution.cc @ 180] 1c (Inline) -------- chrome_child!v8::internal::__RT_impl_Runtime_Apply+0x1c9 [c:\b\build\slave\win\build\src\v8\src\runtime\runtime-function.cc @ 607] 1d 009be1a0 2ea0d4dc chrome_child!v8::internal::Runtime_Apply+0x1d9 [c:\b\build\slave\win\build\src\v8\src\runtime\runtime-function.cc @ 574] 1e 009be1c0 3da16eb2 0x2ea0d4dc 1f 009be200 2ea32c55 0x3da16eb2 20 009be220 2ea30755 0x2ea32c55 21 009be244 2ea168ca 0x2ea30755 22 009be280 695fff83 0x2ea168ca 23 009be2dc 695ff15c chrome_child!v8::internal::Invoke+0x213 [c:\b\build\slave\win\build\src\v8\src\execution.cc @ 129] 24 009be310 695ba6b5 chrome_child!v8::internal::Execution::Call+0x13c [c:\b\build\slave\win\build\src\v8\src\execution.cc @ 180] 25 009be350 6a4dee49 chrome_child!v8::Function::Call+0xb5 [c:\b\build\slave\win\build\src\v8\src\api.cc @ 4028] 26 009be444 696d57be chrome_child!extensions::`anonymous namespace'::ExtensionImpl::Apply+0x2d9 [c:\b\build\slave\win\build\src\extensions\renderer\safe_builtins.cc @ 192] 27 009be478 696a50dd chrome_child!v8::internal::FunctionCallbackArguments::Call+0x7e [c:\b\build\slave\win\build\src\v8\src\arguments.cc @ 34] 28 009be4e4 696a6afd chrome_child!v8::internal::HandleApiCallHelper<0>+0x20d [c:\b\build\slave\win\build\src\v8\src\builtins.cc @ 1078] 29 009be508 2ea0d4dc chrome_child!v8::internal::Builtin_implHandleApiCall+0x2d [c:\b\build\slave\win\build\src\v8\src\builtins.cc @ 1100] 2a 009be528 3da2ad63 0x2ea0d4dc 2b 009be564 2ea0ce7b 0x3da2ad63 2c 009be57c 3dac76d5 0x2ea0ce7b 2d 009be5b4 3dadc8a8 0x3dac76d5 2e 009be5f8 2ea30755 0x3dadc8a8 2f 009be628 2ea168ca 0x2ea30755 30 009be664 695fff83 0x2ea168ca 31 009be6c0 695ff15c chrome_child!v8::internal::Invoke+0x213 [c:\b\build\slave\win\build\src\v8\src\execution.cc @ 129] 32 009be6f4 695ba6b5 chrome_child!v8::internal::Execution::Call+0x13c [c:\b\build\slave\win\build\src\v8\src\execution.cc @ 180] 33 009be734 69f441a3 chrome_child!v8::Function::Call+0xb5 [c:\b\build\slave\win\build\src\v8\src\api.cc @ 4028] 34 009be774 69dd6d64 chrome_child!blink::V8ScriptRunner::callFunction+0xe3 [c:\b\build\slave\win\build\src\third_party\webkit\source\bindings\core\v8\v8scriptrunner.cpp @ 428] 35 009be7d8 69dd6be1 chrome_child!blink::ScriptController::callFunction+0x154 [c:\b\build\slave\win\build\src\third_party\webkit\source\bindings\core\v8\scriptcontroller.cpp @ 165] 36 009be804 69872c21 chrome_child!blink::ScriptController::callFunction+0x41 [c:\b\build\slave\win\build\src\third_party\webkit\source\bindings\core\v8\scriptcontroller.cpp @ 149] 37 009be820 6a4ccc96 chrome_child!blink::WebLocalFrameImpl::callFunctionEvenIfScriptDisabled+0x21 [c:\b\build\slave\win\build\src\third_party\webkit\source\web\weblocalframeimpl.cpp @ 856] 38 009be85c 6a4d9870 chrome_child!extensions::ScriptContext::CallFunction+0xa6 [c:\b\build\slave\win\build\src\extensions\renderer\script_context.cc @ 157] 39 009bea6c 6a4cd7f5 chrome_child!extensions::ModuleSystem::CallModuleMethod+0x2e0 [c:\b\build\slave\win\build\src\extensions\renderer\module_system.cc @ 295] 3a 009beaf8 6a4dfad5 chrome_child!extensions::ScriptContext::OnResponseReceived+0x135 [c:\b\build\slave\win\build\src\extensions\renderer\script_context.cc @ 272] 3b 009beb24 6a4d4d1b chrome_child!extensions::RequestSender::HandleResponse+0x45 [c:\b\build\slave\win\build\src\extensions\renderer\request_sender.cc @ 139] 3c (Inline) -------- chrome_child!DispatchToMethodImpl+0x17 [c:\b\build\slave\win\build\src\base\tuple.h @ 246] 3d (Inline) -------- chrome_child!DispatchToMethod+0x17 [c:\b\build\slave\win\build\src\base\tuple.h @ 253] 3e 009beb80 6a4d5683 chrome_child!ExtensionMsg_Response::Dispatch<extensions::ExtensionHelper,extensions::ExtensionHelper,void,void (__thiscall extensions::ExtensionHelper::*)(int,bool,base::ListValue const &,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &)>+0x5b [c:\b\build\slave\win\build\src\extensions\common\extension_messages.h @ 382] 3f 009bec64 6a1912e7 chrome_child!extensions::ExtensionHelper::OnMessageReceived+0x73 [c:\b\build\slave\win\build\src\extensions\renderer\extension_helper.cc @ 136] 40 009bf074 6a70911e chrome_child!content::RenderViewImpl::OnMessageReceived+0xc7 [c:\b\build\slave\win\build\src\content\renderer\render_view_impl.cc @ 1282] What's happening is accessing the post message property on plugin_ is forcing it to be instantiated--but instantiation fails since it's not supposed to be able to load a local resource. So actually, the lack of a failure on new builds is actually really suspicious. Is the PDF viewer enforcing its own security policy somewhere? I don't see any errors (like I would expect) in the console output when specifying a file:// pdf in a top-level about:blank page, but the embed doesn't seem to be loading either...
,
Mar 15 2018
This is a good question and I don't know the answer. One thing I noticed though is that it looks like the code for checking whether the src can be loaded in blink has been refactored somewhat so my theory is that something in this code has changed (this check may possibly be broken altogether..). I would also add that although blink was throwing up an error here, we could easily work around it. I don't believe that it's important that we set the src= attribute. The plugin does all of its own URL loading, so we could equally pass that URL in under a different attribute. blink does not actually know whether the pepper plugin will make a request to the URL in src so in that sense I think it's a false positive.
,
Mar 15 2018
Raymes: I've been trying to bisect this down a bit. I can confirm that this doesn't seem to be because of https://codereview.chromium.org/2842253002. So at this point, I'm really not sure what made this start working. It was some commit between r313214 and r474107: I checked out r474106 and updated PDF component extension manifest to match the latest changes... and it worked. I wanted to test https://codereview.chromium.org/1645313002 as well to narrow things down a bit, but I wasn't able to build (GN has evolved in ways that aren't backwards compatible, it would seem). I've already burned too much time on this today. We should try to understand what's going on with the embed code here, but that can be followed up separately. On the bright side, that means the fix here is likely safe for merge to M66 (and M65), in that it shouldn't explode there. =)
,
Mar 15 2018
I'm OOO today but I can help next week or sammc@ may be able to take a look. It may be easier to bisect by making a small extension that reproduces the issue (potentially with flash or the test plugin) and using the bisect tool with it.
,
Mar 15 2018
OK, I bisected the embed src error down to https://chromium.googlesource.com/chromium/src/+log/a1ebacdb1bbacbf534ef9f8dc763bc77273fdfd0..3617b0eea7ec74b8e731a23fed2f4070cbc284c4. So perhaps I made an error in my build earlier?
,
Mar 16 2018
After some further thought, I'm pretty sure the actual error message is orthogonal to the host permissions that were previously needed. Even without the console error printed, <embed src="file://..."> doesn't work if the owner document isn't also on a file:// URL: a plugin is never loaded. So the only thing that r474108 changed is that the console error is no longer printed. So something changed somewhere else such that the PDFium component extension doesn't trigger the security check for embedding file:// URLs in non-file:// contexts... but only in the component extension. Furthermore, whatever changed doesn't seem to depend on extension host permissions. So if we can build a copy of the PDFium component extension with chrome://resources/ as the only host permission, we should be able to bisect exactly when that change occurred.
,
Mar 16 2018
I did some digging. I'm wondering if r474108 really is the culprit, though I can't verify it at all. At the very least I suspect r474108 changes more than just whether the console warning is printed. It removes a check from AllowedToLoadObject which queries the SecurityOrigin::CanDisplay (which in turn queries CanRequest). This is the same whitelist that is manipulated by host permissions (see https://cs.chromium.org/chromium/src/extensions/renderer/dispatcher.cc?rcl=2ff761dbda62eda83a4a8378b7741c9cfeaf791b&l=1274). Without that check, AFAICT the only thing that gets queried when loading an <embed> plugin is the CSP. Those checks should pass because we whitelist all origins for object-src. So as far as I can tell from looking into the code everything points toward r474108. With that said, I still believe you when you say the <embed> still doesn't load and that seems weird. If you can't figure this out by my time tomorrow morning I can come into the office and dig into it. I'm still very suspicious of r474108 though. Did you try adding that check back into HTMLPlugInElement.cpp on a ToT build to see if it repros the issue?
,
Mar 16 2018
,
Mar 16 2018
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
,
Mar 17 2018
Ok, on ToT adding back in the GetDocument().GetSecurityOrigin()->CanDisplay(url) check in HTMLPlugInElement::AllowedToLoadObject from r474108 causes the plugin not to load, which is what is expected if r474108 is the culprit. It fails in exactly the same way as in issue 452319 , namely: If the manifest.json includes permissions for file://*/ then local file URLs will load, but network mounts don't load. The exact codepath that is being hit is SecurityOrigin::CanDisplay which checks SchemeRegistry::ShouldTreatURLSchemeAsLocal which returns true for file: URLs. It then queries CanLoadLocalResources() and SecurityPolicy::IsAccessToURLWhiteListed() which both return false when loading a network mount while IsAccessToURLWhiteListed returns true with a local file URL. This is likely due to pattern matching not matching the network mount URLs though I didn't dig that far. With <all_urls> specified IsAccessToURLWhiteListed seems to return true regardless. I wasn't able to check out and build much older versions nearer to sammc's CL (I spent way too long trying without success) but based on the above I feel fairly confident that r474108 is the issue.
,
Mar 19 2018
Does #6 need to be merged, or are there other CLs as well?
,
Mar 19 2018
Comment 31: dcheng@ is out today, but yes, I think we would just need to merge r543030 from comment 6. However, I'm having trouble following the discussion about japhet@'s r474108 and what the implications are. raymes@/sammc@/japhet@: Is it safe to proceed with the merge to M66, or do we need to do something more? (If we can merge, we'd like to do it ASAP.)
,
Mar 19 2018
tldr, I believe it's safe to proceed with a merge to M66. r474108 removed the check that was previously the reason for needing the host permissions. There is an outstanding question about whether that is a regression or intended change. It's hard to know exactly why those checks were there (I'm sure it's steeped in history). I'll file a bug and assign to japhet to follow up on that. But that was landed 10 months ago. So I think we should merge this as a stop-gap. Does that make sense?
,
Mar 19 2018
Filed issue 821613 to follow up on r474108.
,
Mar 20 2018
+falken@ for context on ensuring we don't allow ServiceWorkers on chrome:// URLs in the followup issue 823475.
,
Mar 20 2018
Approving merge to M66. Branch:3359
,
Mar 20 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bf287aafe15dc5f54bb75ff1d4b74c9a9050b6b8 commit bf287aafe15dc5f54bb75ff1d4b74c9a9050b6b8 Author: Daniel Cheng <dcheng@chromium.org> Date: Tue Mar 20 17:55:05 2018 Only grant host permissions for chrome://resources/* to PDF component extension TBR=dcheng@chromium.org (cherry picked from commit d8206c55039dc71555ea92bde3080c8743ad11b8) Bug: 821613 Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation Change-Id: Icdaab0eb77a72a7b6ad0c491424672614c24a9fa Reviewed-on: https://chromium-review.googlesource.com/961727 Reviewed-by: Raymes Khoury <raymes@chromium.org> Reviewed-by: Sam McNally <sammc@chromium.org> Reviewed-by: Lei Zhang <thestig@chromium.org> Commit-Queue: Daniel Cheng <dcheng@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#543030} Reviewed-on: https://chromium-review.googlesource.com/971403 Reviewed-by: Daniel Cheng <dcheng@chromium.org> Cr-Commit-Position: refs/branch-heads/3359@{#349} Cr-Branched-From: 66afc5e5d10127546cc4b98b9117aff588b5e66b-refs/heads/master@{#540276} [modify] https://crrev.com/bf287aafe15dc5f54bb75ff1d4b74c9a9050b6b8/chrome/browser/resources/pdf/manifest.json
,
Mar 20 2018
Even though the exploit doesn't currently work in M65, that's only because the reporter used a M66 and newer exploit to get code execution. If a bug is found in the V8 that ships with M65, we'll have the same bug there. As the manifest change should be safe, I'm going to pre-emptively request a merge in case there's another M65 respin.
,
Mar 23 2018
+awhalley@ for M65 merge review.
,
Mar 23 2018
dcheng@ - now the change in #38 has been out in Beta, mind double checking there's nothing that looks related in the crash reports? Assuming it's good, and it's a safe, easy merge, govind@ - good for 65
,
Mar 26 2018
I compared the top 200 crashes between the previous and current beta versions, and don't see anything that could be obviously related.
,
Mar 26 2018
dcheng@ - thanks! govind@ - Good for 65.
,
Mar 26 2018
Approving merge to M65 branch 3325 based n comments #43, #44 and per offline chat with awhalley@. Note: We won't respin M65 for this but we will pick it up for next respin (if any).
,
Mar 26 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/215a3b7b569cb804b9a0fce0dc4e1b59fbb279a7 commit 215a3b7b569cb804b9a0fce0dc4e1b59fbb279a7 Author: Daniel Cheng <dcheng@chromium.org> Date: Mon Mar 26 18:36:05 2018 Only grant host permissions for chrome://resources/* to PDF component extension TBR=dcheng@chromium.org (cherry picked from commit d8206c55039dc71555ea92bde3080c8743ad11b8) Bug: 821613 Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation Change-Id: Icdaab0eb77a72a7b6ad0c491424672614c24a9fa Reviewed-on: https://chromium-review.googlesource.com/961727 Reviewed-by: Raymes Khoury <raymes@chromium.org> Reviewed-by: Sam McNally <sammc@chromium.org> Reviewed-by: Lei Zhang <thestig@chromium.org> Commit-Queue: Daniel Cheng <dcheng@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#543030} Reviewed-on: https://chromium-review.googlesource.com/981112 Reviewed-by: Daniel Cheng <dcheng@chromium.org> Cr-Commit-Position: refs/branch-heads/3325@{#742} Cr-Branched-From: bc084a8b5afa3744a74927344e304c02ae54189f-refs/heads/master@{#530369} [modify] https://crrev.com/215a3b7b569cb804b9a0fce0dc4e1b59fbb279a7/chrome/browser/resources/pdf/manifest.json
,
Apr 17 2018
,
Jun 20 2018
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 |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by dcheng@chromium.org
, Mar 13 2018