Chromium Code Reviews
Help | Chromium Project | Sign in
(792)

Issue 11313018: Prevent webview tags from navigating outside web-safe schemes. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year, 5 months ago by Charlie Reis
Modified:
1 year, 5 months ago
CC:
chromium-reviews_chromium.org, joi+watch-content_chromium.org, darin-cc_chromium.org, jam
Visibility:
Public.

Description

Prevent webview tags from navigating outside web-safe schemes.

This CL removes protocol handlers and avoids granting capabilities or
bindings to webview processes, which prevents navigations to WebUI,
extension, and file URLs. Web and data URLs are still permitted.

BUG=139397
TEST=Try to visit chrome://settings or other privileged URLs in a <webview>.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=164788

Patch Set 1 #

Patch Set 2 : Fix failing unit tests. #

Patch Set 3 : Remove TODO #

Total comments: 8

Patch Set 4 : Fix review comments #

Total comments: 2

Patch Set 5 : Fix nit #

Unified diffs Side-by-side diffs Delta from patch set Stats (+106 lines, -76 lines) Lint Patch
M chrome/browser/component/navigation_interception/intercept_navigation_resource_throttle.cc View 1 1 chunk +1 line, -4 lines 0 comments ? errors Download
M chrome/browser/profiles/profile_impl_io_data.cc View 1 chunk +8 lines, -4 lines 0 comments ? errors Download
M content/browser/browser_plugin/browser_plugin_embedder.cc View 1 2 1 chunk +4 lines, -1 line 0 comments ? errors Download
M content/browser/browser_plugin/browser_plugin_host_browsertest.cc View 1 chunk +39 lines, -11 lines 0 comments 0 errors Download
M content/browser/renderer_host/render_view_host_impl.h View 1 2 3 1 chunk +1 line, -1 line 0 comments ? errors Download
M content/browser/renderer_host/render_view_host_impl.cc View 1 2 3 12 chunks +45 lines, -27 lines 0 comments ? errors Download
M content/browser/web_contents/web_contents_impl.cc View 1 3 chunks +6 lines, -27 lines 0 comments 0 errors Download
M content/public/browser/render_view_host.h View 1 2 3 4 2 chunks +2 lines, -1 line 0 comments ? errors Download
Commit:

Messages

Total messages: 13
Charlie Reis
Tom, can you take a look at the approach? Fady, can you take a look ...
1 year, 5 months ago #1
Fady Samuel
Test: LGTM
1 year, 5 months ago #2
Charlie Reis
Tom, I uploaded a new CL since the simpler FilterURL change blew up in unit ...
1 year, 5 months ago #3
Charlie Reis
Also adding Will Chan for profile_impl_io_data.cc. (I was able to skip the Interceptor stuff since ...
1 year, 5 months ago #4
willchan
LGTM Btw, all the stuff where you switch passing the render process id to passing ...
1 year, 5 months ago #5
Fady Samuel
Please remove the TODO in BrowserPluginEmbedder::NavigateGuest. Thanks!
1 year, 5 months ago #6
Charlie Reis
On 2012/10/28 07:58:20, willchan wrote: > Btw, all the stuff where you switch passing the ...
1 year, 5 months ago #7
Tom Sepez
LGTM with nits. https://codereview.chromium.org/11313018/diff/16001/chrome/browser/profiles/profile_impl_io_data.cc File chrome/browser/profiles/profile_impl_io_data.cc (right): https://codereview.chromium.org/11313018/diff/16001/chrome/browser/profiles/profile_impl_io_data.cc#newcode490 chrome/browser/profiles/profile_impl_io_data.cc:490: bool is_guest_process = (app_id.find("guest-") != std::string::npos); ...
1 year, 5 months ago #8
Charlie Reis
Thanks, Tom! joth@, can you give an OWNERs review to intercept_navigation_resource_throttle.cc? It's just a method ...
1 year, 5 months ago #9
joth (inactive)
lgtm https://codereview.chromium.org/11313018/diff/24001/content/public/browser/render_view_host.h File content/public/browser/render_view_host.h (right): https://codereview.chromium.org/11313018/diff/24001/content/public/browser/render_view_host.h#newcode40 content/public/browser/render_view_host.h:40: class RenderViewHostDelegate; nit: may as well add RenderProcessHost ...
1 year, 5 months ago #10
Charlie Reis
Thanks! https://codereview.chromium.org/11313018/diff/24001/content/public/browser/render_view_host.h File content/public/browser/render_view_host.h (right): https://codereview.chromium.org/11313018/diff/24001/content/public/browser/render_view_host.h#newcode40 content/public/browser/render_view_host.h:40: class RenderViewHostDelegate; On 2012/10/29 19:36:46, joth wrote: > ...
1 year, 5 months ago #11
I haz the power (commit-bot)
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/creis@chromium.org/11313018/19012
1 year, 5 months ago #12
I haz the power (commit-bot)
1 year, 5 months ago #13
Change committed as 164788
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1280:2d3e6564b7b6