content::HandleViewSource isn't properly enforcing scheme limits for view-source |
||||||
Issue descriptioncontent::HandleViewSource contains a list of schemes allowed for view-source. For any scheme not on the list, it intends to block the navigation by rewriting the passed URL to about:blank and returning false. The motivation appears to be an old issue 26129 . There appear to be several problems with this check at the moment: 1. Pressing ctrl-U on a page with a disallowed scheme still loads fine, because HandleViewSource is never consulted on that path. Repro: go to "data:text/html,<html><body>foo</body></html>" and press ctrl-U. That loads just fine even though "data:" is not an allowed scheme. Same for right-click -> View source. 2. It doesn't work for history navigations. This is because in the renderer, RenderFrameImpl::NavigateInternal generates the navigation request using WebLocalFrameImpl::requestFromHistoryItem(), which reuses the URL that was in the PageState (which wasn't rewritten by HandleViewSource) and ignores the about:blank in commont_params.url. Hence, the request that goes out on the network stack is still view-source:original_url. To repro: close a view-source tab and restore it with ctrl-shift-t. 3. The list of allowed schemes may not be correct. For example, filesystem: is on the list of allowed schemes, but blob: is not, and it probably should be. chrome-extension should certainly be on the list, which I'm fixing in issue 699428 . It's also not clear that schemes like data: should actually be blocked. Issue 26129 seems to be concerned only with javascript:, and even though HandleViewSource mentions data: in a comment, it's unclear whether that was just a misunderstanding. I'm filing this as a followup to issue 699428 , which also has some more related discussion.
,
Mar 20 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7495c6fd9e5f9823396b508f4145450589853391 commit 7495c6fd9e5f9823396b508f4145450589853391 Author: Alex Moshchuk <alexmos@chromium.org> Date: Mon Mar 20 17:10:18 2017 Fix tab restore for view-source Chrome extension pages (Merge to M58). Previously, for non-web-accessible pages, this was blocked by the check in ShouldAllowOpenURL, and generated a DumpWithoutCrashing report because the source SiteInstance ("about:") was not HTTP/HTTPS/extension and the target URL was not a WAR. The source SiteInstance was wrong because content::HandleViewSource disallowed view-source navigations to the chrome-extension scheme and overwrote the destination URL to about:blank. See full analysis in issue 699428. The fix adds chrome-extension to the list of schemes allowed for view-source. It also fixes an issue where the restored view-source tab's visible URL ended up at chrome://bookmarks, rather than view-source:chrome-extension://<bookmark_extension_id>/. BUG= 699428 ,698709,696034,700610 Review-Url: https://codereview.chromium.org/2740013008 Cr-Commit-Position: refs/heads/master@{#457582} (cherry picked from commit 94875b3b4d4a1fc047b99214f30078f3780259f0) Review-Url: https://codereview.chromium.org/2760023002 . Cr-Commit-Position: refs/branch-heads/3029@{#301} Cr-Branched-From: 939b32ee5ba05c396eef3fd992822fcca9a2e262-refs/heads/master@{#454471} [modify] https://crrev.com/7495c6fd9e5f9823396b508f4145450589853391/chrome/browser/chrome_content_browser_client.cc [modify] https://crrev.com/7495c6fd9e5f9823396b508f4145450589853391/chrome/browser/chrome_content_browser_client.h [modify] https://crrev.com/7495c6fd9e5f9823396b508f4145450589853391/chrome/browser/tab_contents/view_source_browsertest.cc [modify] https://crrev.com/7495c6fd9e5f9823396b508f4145450589853391/content/browser/browser_url_handler_impl.cc [modify] https://crrev.com/7495c6fd9e5f9823396b508f4145450589853391/content/public/browser/content_browser_client.cc [modify] https://crrev.com/7495c6fd9e5f9823396b508f4145450589853391/content/public/browser/content_browser_client.h
,
Sep 21 2017
,
Sep 24
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue. Sorry for the inconvenience if the bug really should have been left as Available. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Sep 27
Still needs to be done. The repro (1) from #0 still works, and (3) still applies: https://cs.chromium.org/chromium/src/content/browser/browser_url_handler_impl.cc?l=28&rcl=5a5b7a65eaa6c479c9a325b2775270172cc11b2e
,
Nov 7
|
||||||
►
Sign in to add a comment |
||||||
Comment 1 by bugdroid1@chromium.org
, Mar 16 2017