This is essentially the same as  Issue 705206 , but introduces the observation that Content-Security-Policy is bypassed in this scenario.

Given the user-interaction requirement, this should probably be Severity_Low.

Oops, this is worse than I thought.

This bug "introduces" XSS to a site which wasn't possible otherwise.

PoC
https://vuln.shhnjk.com/xssable.php?xss=%3Cimg%20src=javascript:alert(1)%3E%3C!--

Please change the summary accordingly :)

Does this really affect iOS? We don't have Blink/our own CSP there. Please correct me if I'm wrong.

andypaicu, can you please take a look? Thanks.

Good point, view-source: isn't available on iOS.

Surgical CL:
https://chromium-review.googlesource.com/c/chromium/src/+/900099

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4f811848ab21dbbe39f64f82f61aa0e41c14a227

commit 4f811848ab21dbbe39f64f82f61aa0e41c14a227
Author: Eric Lawrence <elawrence@chromium.org>
Date: Tue Feb 06 17:37:23 2018

Sanitize JavaScript links in HTMLViewSourceDocument

To limit mischief, replace the target of JavaScript-scheme links in
HTMLViewSourceDocument with about:blank.

Bug:  705206 ,  808407 
Change-Id: I185006d0cb29caabcd08dd9d5b9324357c79efaa
Reviewed-on: https://chromium-review.googlesource.com/900099
Reviewed-by: Mike West <mkwst@chromium.org>
Commit-Queue: Eric Lawrence <elawrence@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534705}
[modify] https://crrev.com/4f811848ab21dbbe39f64f82f61aa0e41c14a227/third_party/WebKit/Source/core/html/HTMLViewSourceDocument.cpp

From the code review: 

"Let's keep the patch small and merge it back to 65. That said, I think it's worth putting together a followup to harden `view-source:` a bit more. Perhaps we could poke at `Document::SetIsViewSource` to move the document into an opaque origin, and `Document::CanExecuteScripts` to block script execution for `Document`s where `IsViewSource()` is true? WDYT? I'd be fine with adding a CSP as well. Might as well add all the layers. :)

Adding a block to CanExecuteScripts seems trivial. Adding CSP is also easy:

  HTMLMetaElement* csp = HTMLMetaElement::Create(*this);
  csp->setAttribute(http_equivAttr, "Content-Security-Policy");
  csp->setAttribute(contentAttr, "default-src 'none'");
  head->ParserAppendChild(csp);

Switching to a unique origin isn't something I've done before but probably isn't too hard. 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bdad902d8163ca5a723d930d1f22bea61669dd72

commit bdad902d8163ca5a723d930d1f22bea61669dd72
Author: Eric Lawrence <elawrence@chromium.org>
Date: Wed Feb 07 19:06:40 2018

Sanitize JavaScript links in HTMLViewSourceDocument (M65 merge)

To limit mischief, replace the target of JavaScript-scheme links in
HTMLViewSourceDocument with about:blank.

Bug:  705206 ,  808407 
Change-Id: I185006d0cb29caabcd08dd9d5b9324357c79efaa
Reviewed-on: https://chromium-review.googlesource.com/900099
Reviewed-by: Mike West <mkwst@chromium.org>
Commit-Queue: Eric Lawrence <elawrence@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#534705}(cherry picked from commit 4f811848ab21dbbe39f64f82f61aa0e41c14a227)
Reviewed-on: https://chromium-review.googlesource.com/907408
Reviewed-by: Eric Lawrence <elawrence@chromium.org>
Cr-Commit-Position: refs/branch-heads/3325@{#366}
Cr-Branched-From: bc084a8b5afa3744a74927344e304c02ae54189f-refs/heads/master@{#530369}
[modify] https://crrev.com/bdad902d8163ca5a723d930d1f22bea61669dd72/third_party/WebKit/Source/core/html/HTMLViewSourceDocument.cpp

Please mark security bugs as fixed as soon as the fix lands, and before requesting merges. This update is based on the merge- labels applied to this issue. Please reopen if this update was incorrect.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Still more work to do, per #7.

I can see that this is fixed in Chrome 65. Can I talk about this bug publicly? Or should I wait till defense-in-depth mitigations are pushed?

The broader Defense-in-Depth fix breaks a *ton* of tests, so it will need considerably more work to land:

https://chromium-review.googlesource.com/c/chromium/src/+/917121

failures:
ViewSourceTest.CrossSiteSubframe
ViewSourceTest.JavaScriptURISanitized
ViewSourceTest.ViewSourceCrossProcessAndBack
ViewExtensionSourceTest.ViewSourceTabRestore
ExtensionSettingsUIBrowserTest.ViewSource
ViewSourceTest.HttpPostInMainframe
ViewSourceTest.HttpPostInSubframe
ViewSourceTest.NavigationOmitsReferrer
RenderFrameHostManagerTest.RendererDebugURLsDontSwap

 Issue 833916  has been merged into this issue.

 Issue 833916  simply noted that View-Source was no longer in its own origin. Comment #2 in that issue helps explain why we probably need to land *both* disabling script execution AND opaque-origin in the same CL.

I guess elawrence@ already left Google. This bug needs new owner.

Unfortunately, I won't be able to work on this further in the foreseeable future. I think we can call the original bug "Fixed", but we may want further hardening. 

Could anyone mark this bug as fixed and create another bug for DiD hardening if required?

Filed hardening bug at bug 868488, closing this one as fixed.

(also added Eric back as owner of this now fixed bug since he did all the hard work here)

Hello! Thanks for the report, but the VRP declined to reward, given the level of user interaction required.

This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot