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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 years, 8 months ago by Charlie Reis (OOO until Jul 6)
Modified:
2 years, 8 months ago
CC:
chromium-reviews, 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 #

Messages

Total messages: 13 (0 generated)
Charlie Reis (OOO until Jul 6)
Tom, can you take a look at the approach? Fady, can you take a look ...
2 years, 8 months ago (2012-10-27 00:00:08 UTC) #1
Fady Samuel
Test: LGTM
2 years, 8 months ago (2012-10-27 00:06:01 UTC) #2
Charlie Reis (OOO until Jul 6)
Tom, I uploaded a new CL since the simpler FilterURL change blew up in unit ...
2 years, 8 months ago (2012-10-27 01:43:41 UTC) #3
Charlie Reis (OOO until Jul 6)
Also adding Will Chan for profile_impl_io_data.cc. (I was able to skip the Interceptor stuff since ...
2 years, 8 months ago (2012-10-27 01:46:05 UTC) #4
willchan no longer on Chromium
LGTM Btw, all the stuff where you switch passing the render process id to passing ...
2 years, 8 months ago (2012-10-28 07:58:20 UTC) #5
Fady Samuel
Please remove the TODO in BrowserPluginEmbedder::NavigateGuest. Thanks!
2 years, 8 months ago (2012-10-29 00:48:09 UTC) #6
Charlie Reis (OOO until Jul 6)
On 2012/10/28 07:58:20, willchan wrote: > Btw, all the stuff where you switch passing the ...
2 years, 8 months ago (2012-10-29 16:36:30 UTC) #7
Tom Sepez (OOO til 13 jul)
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); ...
2 years, 8 months ago (2012-10-29 17:59:08 UTC) #8
Charlie Reis (OOO until Jul 6)
Thanks, Tom! joth@, can you give an OWNERs review to intercept_navigation_resource_throttle.cc? It's just a method ...
2 years, 8 months ago (2012-10-29 18:18:31 UTC) #9
joth
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 ...
2 years, 8 months ago (2012-10-29 19:36:46 UTC) #10
Charlie Reis (OOO until Jul 6)
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: > ...
2 years, 8 months ago (2012-10-29 19:41:42 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/creis@chromium.org/11313018/19012
2 years, 8 months ago (2012-10-29 19:41:57 UTC) #12
commit-bot: I haz the power
2 years, 8 months ago (2012-10-30 00:20:10 UTC) #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 1f9106d