Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1184)

Side by Side Diff: chrome/browser/extensions/chrome_content_browser_client_extensions_part.cc

Issue 2454563003: Fix web accessible resource checks in ShouldAllowOpenURL (Closed)
Patch Set: Round 2 of Devlin's comments Created 4 years, 1 month ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
OLDNEW
1 // Copyright 2014 The Chromium Authors. All rights reserved. 1 // Copyright 2014 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "chrome/browser/extensions/chrome_content_browser_client_extensions_par t.h" 5 #include "chrome/browser/extensions/chrome_content_browser_client_extensions_par t.h"
6 6
7 #include <stddef.h> 7 #include <stddef.h>
8 8
9 #include <set> 9 #include <set>
10 10
11 #include "base/command_line.h" 11 #include "base/command_line.h"
12 #include "base/debug/alias.h"
13 #include "base/debug/dump_without_crashing.h"
14 #include "base/metrics/histogram_macros.h"
12 #include "chrome/browser/browser_process.h" 15 #include "chrome/browser/browser_process.h"
13 #include "chrome/browser/extensions/extension_service.h" 16 #include "chrome/browser/extensions/extension_service.h"
14 #include "chrome/browser/extensions/extension_web_ui.h" 17 #include "chrome/browser/extensions/extension_web_ui.h"
15 #include "chrome/browser/extensions/extension_webkit_preferences.h" 18 #include "chrome/browser/extensions/extension_webkit_preferences.h"
16 #include "chrome/browser/media_galleries/fileapi/media_file_system_backend.h" 19 #include "chrome/browser/media_galleries/fileapi/media_file_system_backend.h"
17 #include "chrome/browser/profiles/profile.h" 20 #include "chrome/browser/profiles/profile.h"
18 #include "chrome/browser/profiles/profile_io_data.h" 21 #include "chrome/browser/profiles/profile_io_data.h"
19 #include "chrome/browser/profiles/profile_manager.h" 22 #include "chrome/browser/profiles/profile_manager.h"
20 #include "chrome/browser/renderer_host/chrome_extension_message_filter.h" 23 #include "chrome/browser/renderer_host/chrome_extension_message_filter.h"
21 #include "chrome/browser/sync_file_system/local/sync_file_system_backend.h" 24 #include "chrome/browser/sync_file_system/local/sync_file_system_backend.h"
(...skipping 54 matching lines...) Expand 10 before | Expand all | Expand 10 after
76 // below. Extension, and isolated apps require different privileges to be 79 // below. Extension, and isolated apps require different privileges to be
77 // granted to their RenderProcessHosts. This classification allows us to make 80 // granted to their RenderProcessHosts. This classification allows us to make
78 // sure URLs are served by hosts with the right set of privileges. 81 // sure URLs are served by hosts with the right set of privileges.
79 enum RenderProcessHostPrivilege { 82 enum RenderProcessHostPrivilege {
80 PRIV_NORMAL, 83 PRIV_NORMAL,
81 PRIV_HOSTED, 84 PRIV_HOSTED,
82 PRIV_ISOLATED, 85 PRIV_ISOLATED,
83 PRIV_EXTENSION, 86 PRIV_EXTENSION,
84 }; 87 };
85 88
89 // Specifies reasons why web-accessible resource checks in ShouldAllowOpenURL
90 // might fail.
91 //
92 // This enum backs an UMA histogram. The order of existing values
93 // should not be changed, and new values should only be added before
94 // FAILURE_LAST.
95 enum ShouldAllowOpenURLFailureReason {
96 FAILURE_FILE_SYSTEM_URL = 0,
97 FAILURE_BLOB_URL,
98 FAILURE_SCHEME_NOT_HTTP_OR_HTTPS_OR_EXTENSION,
99 FAILURE_RESOURCE_NOT_WEB_ACCESSIBLE,
100 FAILURE_LAST,
101 };
102
86 RenderProcessHostPrivilege GetPrivilegeRequiredByUrl( 103 RenderProcessHostPrivilege GetPrivilegeRequiredByUrl(
87 const GURL& url, 104 const GURL& url,
88 ExtensionRegistry* registry) { 105 ExtensionRegistry* registry) {
89 // Default to a normal renderer cause it is lower privileged. This should only 106 // Default to a normal renderer cause it is lower privileged. This should only
90 // occur if the URL on a site instance is either malformed, or uninitialized. 107 // occur if the URL on a site instance is either malformed, or uninitialized.
91 // If it is malformed, then there is no need for better privileges anyways. 108 // If it is malformed, then there is no need for better privileges anyways.
92 // If it is uninitialized, but eventually settles on being an a scheme other 109 // If it is uninitialized, but eventually settles on being an a scheme other
93 // than normal webrenderer, the navigation logic will correct us out of band 110 // than normal webrenderer, the navigation logic will correct us out of band
94 // anyways. 111 // anyways.
95 if (!url.is_valid()) 112 if (!url.is_valid())
(...skipping 94 matching lines...) Expand 10 before | Expand all | Expand 10 after
190 DCHECK(origin.SchemeIs(extensions::kExtensionScheme)); 207 DCHECK(origin.SchemeIs(extensions::kExtensionScheme));
191 208
192 if (IsIllegalOrigin(resource_context, child_id, origin)) { 209 if (IsIllegalOrigin(resource_context, child_id, origin)) {
193 // TODO(ananta): Find a way to specify the right error code here. 210 // TODO(ananta): Find a way to specify the right error code here.
194 callback.Run(false, 0); 211 callback.Run(false, 0);
195 } else { 212 } else {
196 callback.Run(true, 0); 213 callback.Run(true, 0);
197 } 214 }
198 } 215 }
199 216
217 void RecordShowAllowOpenURLFailure(ShouldAllowOpenURLFailureReason reason) {
218 UMA_HISTOGRAM_ENUMERATION("Extensions.ShouldAllowOpenURL.Failure", reason,
219 FAILURE_LAST);
220 }
221
200 } // namespace 222 } // namespace
201 223
202 ChromeContentBrowserClientExtensionsPart:: 224 ChromeContentBrowserClientExtensionsPart::
203 ChromeContentBrowserClientExtensionsPart() { 225 ChromeContentBrowserClientExtensionsPart() {
204 } 226 }
205 227
206 ChromeContentBrowserClientExtensionsPart:: 228 ChromeContentBrowserClientExtensionsPart::
207 ~ChromeContentBrowserClientExtensionsPart() { 229 ~ChromeContentBrowserClientExtensionsPart() {
208 } 230 }
209 231
(...skipping 279 matching lines...) Expand 10 before | Expand all | Expand 10 after
489 const Extension* extension = 511 const Extension* extension =
490 extension_info_map->extensions().GetExtensionOrAppByURL(first_party_url); 512 extension_info_map->extensions().GetExtensionOrAppByURL(first_party_url);
491 // Don't allow a service worker for an extension url with no extension (this 513 // Don't allow a service worker for an extension url with no extension (this
492 // could happen in the case of, e.g., an unloaded extension). 514 // could happen in the case of, e.g., an unloaded extension).
493 return extension != nullptr; 515 return extension != nullptr;
494 } 516 }
495 517
496 // static 518 // static
497 bool ChromeContentBrowserClientExtensionsPart::ShouldAllowOpenURL( 519 bool ChromeContentBrowserClientExtensionsPart::ShouldAllowOpenURL(
498 content::SiteInstance* site_instance, 520 content::SiteInstance* site_instance,
499 const GURL& from_url,
500 const GURL& to_url, 521 const GURL& to_url,
501 bool* result) { 522 bool* result) {
502 DCHECK(result); 523 DCHECK(result);
503 524
525 // Using url::Origin is important to properly handle blob: and filesystem:
526 // URLs.
527 url::Origin to_origin(to_url);
528 if (to_origin.scheme() != kExtensionScheme) {
529 // We're not responsible for protecting this resource. Note that hosted
530 // apps fall into this category.
531 return false;
532 }
533
504 // Do not allow pages from the web or other extensions navigate to 534 // Do not allow pages from the web or other extensions navigate to
505 // non-web-accessible extension resources. 535 // non-web-accessible extension resources.
506 if (to_url.SchemeIs(kExtensionScheme) &&
507 (from_url.SchemeIsHTTPOrHTTPS() || from_url.SchemeIs(kExtensionScheme))) {
508 Profile* profile = Profile::FromBrowserContext(
509 site_instance->GetProcess()->GetBrowserContext());
510 ExtensionRegistry* registry = ExtensionRegistry::Get(profile);
511 if (!registry) {
512 *result = true;
513 return true;
514 }
515 const Extension* extension =
516 registry->enabled_extensions().GetExtensionOrAppByURL(to_url);
517 if (!extension) {
518 *result = true;
519 return true;
520 }
521 const Extension* from_extension =
522 registry->enabled_extensions().GetExtensionOrAppByURL(
523 site_instance->GetSiteURL());
524 if (from_extension && from_extension->id() == extension->id()) {
525 *result = true;
526 return true;
527 }
528 536
529 if (!WebAccessibleResourcesInfo::IsResourceWebAccessible( 537 ExtensionRegistry* registry =
530 extension, to_url.path())) { 538 ExtensionRegistry::Get(site_instance->GetBrowserContext());
531 *result = false; 539 const Extension* to_extension =
532 return true; 540 registry->enabled_extensions().GetByID(to_origin.host());
533 } 541 if (!to_extension) {
542 *result = true;
543 return true;
534 } 544 }
535 return false; 545
546 GURL site_url(site_instance->GetSiteURL());
547 const Extension* from_extension =
548 registry->enabled_extensions().GetExtensionOrAppByURL(site_url);
549 if (from_extension && from_extension == to_extension) {
550 *result = true;
551 return true;
552 }
553
554 // Blob and filesystem URLs are never considered web-accessible. See
555 // https://crbug.com/656752.
556 if (to_url.SchemeIsFileSystem() || to_url.SchemeIsBlob()) {
557 if (to_url.SchemeIsFileSystem())
558 RecordShowAllowOpenURLFailure(FAILURE_FILE_SYSTEM_URL);
559 else
560 RecordShowAllowOpenURLFailure(FAILURE_BLOB_URL);
561
562 // TODO(alexmos): Temporary instrumentation to find any regressions for
563 // this blocking. Remove after verifying that this is not breaking any
564 // legitimate use cases.
565 char site_url_copy[256];
566 base::strlcpy(site_url_copy, site_url.spec().c_str(),
567 arraysize(site_url_copy));
568 base::debug::Alias(&site_url_copy);
569 char to_origin_copy[256];
570 base::strlcpy(to_origin_copy, to_origin.Serialize().c_str(),
571 arraysize(to_origin_copy));
572 base::debug::Alias(&to_origin_copy);
573 base::debug::DumpWithoutCrashing();
574
575 *result = false;
576 return true;
577 }
578
579 if (WebAccessibleResourcesInfo::IsResourceWebAccessible(to_extension,
580 to_url.path())) {
581 *result = true;
582 return true;
583 }
584
585 if (!site_url.SchemeIsHTTPOrHTTPS() && !site_url.SchemeIs(kExtensionScheme)) {
586 RecordShowAllowOpenURLFailure(
587 FAILURE_SCHEME_NOT_HTTP_OR_HTTPS_OR_EXTENSION);
588
589 // TODO(alexmos): Previous version of this function skipped the
590 // web-accessible resource checks in this case. Collect data to catch
591 // any regressions, and then remove this.
592 char site_url_copy[256];
593 base::strlcpy(site_url_copy, site_url.spec().c_str(),
594 arraysize(site_url_copy));
595 base::debug::Alias(&site_url_copy);
596 char to_origin_copy[256];
597 base::strlcpy(to_origin_copy, to_origin.Serialize().c_str(),
598 arraysize(to_origin_copy));
599 base::debug::Alias(&to_origin_copy);
600 base::debug::DumpWithoutCrashing();
601 } else {
602 RecordShowAllowOpenURLFailure(FAILURE_RESOURCE_NOT_WEB_ACCESSIBLE);
603 }
604
605 *result = false;
606 return true;
536 } 607 }
537 608
538 // static 609 // static
539 std::unique_ptr<content::VpnServiceProxy> 610 std::unique_ptr<content::VpnServiceProxy>
540 ChromeContentBrowserClientExtensionsPart::GetVpnServiceProxy( 611 ChromeContentBrowserClientExtensionsPart::GetVpnServiceProxy(
541 content::BrowserContext* browser_context) { 612 content::BrowserContext* browser_context) {
542 #if defined(OS_CHROMEOS) 613 #if defined(OS_CHROMEOS)
543 chromeos::VpnService* vpn_service = 614 chromeos::VpnService* vpn_service =
544 chromeos::VpnServiceFactory::GetForBrowserContext(browser_context); 615 chromeos::VpnServiceFactory::GetForBrowserContext(browser_context);
545 if (!vpn_service) 616 if (!vpn_service)
(...skipping 141 matching lines...) Expand 10 before | Expand all | Expand 10 after
687 command_line->AppendSwitch(switches::kExtensionProcess); 758 command_line->AppendSwitch(switches::kExtensionProcess);
688 } 759 }
689 } 760 }
690 761
691 void ChromeContentBrowserClientExtensionsPart::ResourceDispatcherHostCreated() { 762 void ChromeContentBrowserClientExtensionsPart::ResourceDispatcherHostCreated() {
692 content::ResourceDispatcherHost::Get()->RegisterInterceptor( 763 content::ResourceDispatcherHost::Get()->RegisterInterceptor(
693 "Origin", kExtensionScheme, base::Bind(&OnHttpHeaderReceived)); 764 "Origin", kExtensionScheme, base::Bind(&OnHttpHeaderReceived));
694 } 765 }
695 766
696 } // namespace extensions 767 } // namespace extensions
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698