diff options
author | tsepez@chromium.org <tsepez@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-07-17 09:43:40 +0000 |
---|---|---|
committer | tsepez@chromium.org <tsepez@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-07-17 09:43:40 +0000 |
commit | 5d7495738335bbd2014c4eb3e8df998530f0e1db (patch) | |
tree | bc35c528028ff0c82b700dc25d054ac5e56dd137 | |
parent | 58e1a27d4716dab191ab0a23328686841801c6be (diff) | |
download | chromium_src-5d7495738335bbd2014c4eb3e8df998530f0e1db.zip chromium_src-5d7495738335bbd2014c4eb3e8df998530f0e1db.tar.gz chromium_src-5d7495738335bbd2014c4eb3e8df998530f0e1db.tar.bz2 |
Allow view-source of pages fully blocked by blinks XSS filter.
Unlike the other kinds of errors which are detected earlier in navigation
(SSL certs, etc), when the Blink reflected XSS filter encounters an
XSS and the page needs to be blocked (per the server's request), we
already are have a commited navigation, and are well past the point
where interstitials and the like would do us any good.
Consequently, blink just aborts the load, and schedules a navigation to
data:, with history replacement enabled, so that the offending entry
is lost (note https://codereview.chromium.org/301163006/ changes this
behaviour blink-side to add to the back-forward list).
This is less than ideal when a webmaster would like to do a view-source on
the offending page so as to diagnose the cause, so what I've done is to set
up a way to flag the offending entry when the reflection is detected.
I'd really like to just continue with navigating to data:, rather than
trying to deal with the UX issue -- there's nothing to be done, and
screaming about XSS isn't helpful to the user -- and we aren't going
to ever add a "revisit the page with protection disabled" option neither.
So, when a block is detected, we make an IPC call to flag the current entry
in the navigation controller. The navigation then continues to data:,.
When we encounter a view-source on the data:, page URL, we check if the
previous page was explicitly flagged prior to the block. If so, show its
source instead.
Review URL: https://codereview.chromium.org/304313003
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@283728 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r-- | chrome/browser/ui/browser_commands.cc | 11 | ||||
-rw-r--r-- | content/browser/frame_host/navigation_entry_impl.cc | 6 | ||||
-rw-r--r-- | content/browser/frame_host/navigation_entry_impl.h | 11 | ||||
-rw-r--r-- | content/browser/web_contents/web_contents_impl.cc | 21 | ||||
-rw-r--r-- | content/browser/web_contents/web_contents_impl.h | 1 | ||||
-rw-r--r-- | content/common/frame_messages.h | 6 | ||||
-rw-r--r-- | content/public/browser/navigation_entry.h | 3 | ||||
-rw-r--r-- | content/renderer/render_frame_impl.cc | 7 | ||||
-rw-r--r-- | content/renderer/render_frame_impl.h | 3 |
9 files changed, 69 insertions, 0 deletions
diff --git a/chrome/browser/ui/browser_commands.cc b/chrome/browser/ui/browser_commands.cc index 479e4e8..beb26a6 100644 --- a/chrome/browser/ui/browser_commands.cc +++ b/chrome/browser/ui/browser_commands.cc @@ -1157,6 +1157,17 @@ void ViewSource(Browser* browser, WebContents* contents) { if (!entry) return; + // The URL "data:," is a special case, since Blink uses it when it wants to + // show a "blocked page" from its reflected XSS filter. When the XSS filter + // triggers, the current entry gets marked as containing an XSS, and then a + // new navigation to "data:," occurs on top of it. Showing that page in place + // of the "data:," URL permits examination of the cause of the reflection. + if (entry->GetURL() == GURL("data:,")) { + NavigationEntry* previous = contents->GetController().GetEntryAtOffset(-1); + if (previous && previous->GetXssDetected()) + entry = previous; + } + ViewSource(browser, contents, entry->GetURL(), entry->GetPageState()); } diff --git a/content/browser/frame_host/navigation_entry_impl.cc b/content/browser/frame_host/navigation_entry_impl.cc index b021d5e..5f82ea5 100644 --- a/content/browser/frame_host/navigation_entry_impl.cc +++ b/content/browser/frame_host/navigation_entry_impl.cc @@ -53,6 +53,7 @@ NavigationEntryImpl::NavigationEntryImpl() should_replace_entry_(false), should_clear_history_list_(false), can_load_local_resources_(false), + xss_detected_(false), frame_tree_node_id_(-1) { } @@ -82,6 +83,7 @@ NavigationEntryImpl::NavigationEntryImpl(SiteInstanceImpl* instance, should_replace_entry_(false), should_clear_history_list_(false), can_load_local_resources_(false), + xss_detected_(false), frame_tree_node_id_(-1) { } @@ -310,6 +312,10 @@ bool NavigationEntryImpl::GetCanLoadLocalResources() const { return can_load_local_resources_; } +bool NavigationEntryImpl::GetXssDetected() const { + return xss_detected_; +} + void NavigationEntryImpl::SetFrameToNavigate(const std::string& frame_name) { frame_to_navigate_ = frame_name; } diff --git a/content/browser/frame_host/navigation_entry_impl.h b/content/browser/frame_host/navigation_entry_impl.h index 613f45e..c162d22 100644 --- a/content/browser/frame_host/navigation_entry_impl.h +++ b/content/browser/frame_host/navigation_entry_impl.h @@ -89,6 +89,7 @@ class CONTENT_EXPORT NavigationEntryImpl virtual void SetRedirectChain(const std::vector<GURL>& redirects) OVERRIDE; virtual const std::vector<GURL>& GetRedirectChain() const OVERRIDE; virtual bool IsRestored() const OVERRIDE; + virtual bool GetXssDetected() const OVERRIDE; // Once a navigation entry is committed, we should no longer track several // pieces of non-persisted state, as documented on the members below. @@ -218,6 +219,12 @@ class CONTENT_EXPORT NavigationEntryImpl frame_tree_node_id_ = frame_tree_node_id; } + // Called when an XSS detected by Blink's XSSAuditor caused the page to + // be blocked. + void set_xss_detected(bool xss_detected) { + xss_detected_ = xss_detected; + } + private: // WARNING WARNING WARNING WARNING WARNING WARNING WARNING WARNING WARNING // Session/Tab restore save portions of this class so that it can be recreated @@ -322,6 +329,10 @@ class CONTENT_EXPORT NavigationEntryImpl // value is not needed after the entry commits and is not persisted. bool can_load_local_resources_; + // Set when this entry was blocked by Blink's XSSAuditor. + // TODO(tsepez): persist xss_detected_ (see WARNING section above). + bool xss_detected_; + // If not empty, the name of the frame to navigate. This field is not // persisted, because it is currently only used in tests. std::string frame_to_navigate_; diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc index 780c6b9..0471f0b 100644 --- a/content/browser/web_contents/web_contents_impl.cc +++ b/content/browser/web_contents/web_contents_impl.cc @@ -508,6 +508,7 @@ bool WebContentsImpl::OnMessageReceived(RenderViewHost* render_view_host, OnDomOperationResponse) IPC_MESSAGE_HANDLER(FrameHostMsg_DidChangeThemeColor, OnThemeColorChanged) + IPC_MESSAGE_HANDLER(FrameHostMsg_DidDetectXSS, OnDidDetectXSS) IPC_MESSAGE_HANDLER(FrameHostMsg_DidFinishDocumentLoad, OnDocumentLoadedInFrame) IPC_MESSAGE_HANDLER(FrameHostMsg_DidFinishLoad, OnDidFinishLoad) @@ -2578,6 +2579,26 @@ void WebContentsImpl::OnDidRunInsecureContent( GetController().GetBrowserContext()); } + +void WebContentsImpl::OnDidDetectXSS(int32 page_id, + const GURL& url, + bool blocked_entire_page) { + if (!blocked_entire_page) + return; + + int entry_index = controller_.GetEntryIndexWithPageID( + GetRenderViewHost()->GetSiteInstance(), page_id); + if (entry_index < 0) + return; + + NavigationEntryImpl* entry = NavigationEntryImpl::FromNavigationEntry( + controller_.GetEntryAtIndex(entry_index)); + if (!entry) + return; + + entry->set_xss_detected(true); +} + void WebContentsImpl::OnDocumentLoadedInFrame() { CHECK(render_frame_message_source_); CHECK(!render_view_message_source_); diff --git a/content/browser/web_contents/web_contents_impl.h b/content/browser/web_contents/web_contents_impl.h index 30c4eee..b001642 100644 --- a/content/browser/web_contents/web_contents_impl.h +++ b/content/browser/web_contents/web_contents_impl.h @@ -711,6 +711,7 @@ class CONTENT_EXPORT WebContentsImpl void OnDidDisplayInsecureContent(); void OnDidRunInsecureContent(const std::string& security_origin, const GURL& target_url); + void OnDidDetectXSS(int32 page_id, const GURL& url, bool blocked_entire_page); void OnDocumentLoadedInFrame(); void OnDidFinishLoad(const GURL& url); void OnDidStartLoading(bool to_different_document); diff --git a/content/common/frame_messages.h b/content/common/frame_messages.h index 5803d6b..66ab428 100644 --- a/content/common/frame_messages.h +++ b/content/common/frame_messages.h @@ -584,6 +584,12 @@ IPC_MESSAGE_ROUTED1(FrameHostMsg_ForwardInputEvent, // user right clicked. IPC_MESSAGE_ROUTED1(FrameHostMsg_ContextMenu, content::ContextMenuParams) +// Sent when the renderer detects an XSS in a page. +IPC_MESSAGE_ROUTED3(FrameHostMsg_DidDetectXSS, + int32 /* page_id */, + GURL /* url */, + bool /* blocked entire page */) + // Initial drawing parameters for a child frame that has been swapped out to // another process. IPC_MESSAGE_ROUTED2(FrameHostMsg_InitializeChildFrame, diff --git a/content/public/browser/navigation_entry.h b/content/public/browser/navigation_entry.h index 6d69b0d..11b5c0f 100644 --- a/content/public/browser/navigation_entry.h +++ b/content/public/browser/navigation_entry.h @@ -217,6 +217,9 @@ class NavigationEntry { // True if this entry is restored and hasn't been loaded. virtual bool IsRestored() const = 0; + + // Whether an XSS was detected by Blink's XSSAuditor for this entry. + virtual bool GetXssDetected() const = 0; }; } // namespace content diff --git a/content/renderer/render_frame_impl.cc b/content/renderer/render_frame_impl.cc index eb7370c..8470b30 100644 --- a/content/renderer/render_frame_impl.cc +++ b/content/renderer/render_frame_impl.cc @@ -2632,6 +2632,13 @@ void RenderFrameImpl::didRunInsecureContent( target)); } +void RenderFrameImpl::didDetectXSS(blink::WebLocalFrame* frame, + const blink::WebURL& url, + bool blocked_entire_page) { + Send(new FrameHostMsg_DidDetectXSS(routing_id_, render_view_->page_id_, url, + blocked_entire_page)); +} + void RenderFrameImpl::didAbortLoading(blink::WebLocalFrame* frame) { DCHECK(!frame_ || frame_ == frame); #if defined(ENABLE_PLUGINS) diff --git a/content/renderer/render_frame_impl.h b/content/renderer/render_frame_impl.h index d1a3303..e9e8270 100644 --- a/content/renderer/render_frame_impl.h +++ b/content/renderer/render_frame_impl.h @@ -350,6 +350,9 @@ class CONTENT_EXPORT RenderFrameImpl virtual void didRunInsecureContent(blink::WebLocalFrame* frame, const blink::WebSecurityOrigin& origin, const blink::WebURL& target); + virtual void didDetectXSS(blink::WebLocalFrame* frame, + const blink::WebURL& url, + bool blocked_entire_page); virtual void didAbortLoading(blink::WebLocalFrame* frame); virtual void didCreateScriptContext(blink::WebLocalFrame* frame, v8::Handle<v8::Context> context, |