Thanks for reporting, g_google@flamescape.com!

+ianwen@, could you help triage this issue? It looks similar to  bug 481015 .  Please feel free to suggest other owner. 

Hmm this is windows 10?

I am not sure who is working on bookmark for desktop now. Since it's a security issue, I would reassign it to felt@ and let her decide?

it can be reproduced on linux as well. I haven't tested it on mobile, but I guess this issue is not specific to a particular OS.
+felt@, could you suggest a owner? 

Re #13: Feature teams are responsible for fixing the bugs in their areas of code. The security team is just here to triage and help provide advice if you need it. We wouldn't be able to scale up to fix all security bugs in all features. So in this case, someone on the bookmarks team ought to investigate this. :)

shrike@ - it looks like you have been fixing bookmarks-related bugs, do you know who owns bookmarks now?

I've only been doing UI work on bookmarks on the Mac. I'm not sure who would be a good person for this fix, but taking a stab at rdevlin.cronin@.

@5, @8 it looks like sky@ owns bookmarks (components/bookmarks and c/b/bookmarks).  Over to him for triage.

sky: Uh oh! This issue still open and hasn't been updated in the last 14 days. This is a serious vulnerability, and we want to ensure that there's progress. Could you please leave an update with the current status and any potential blockers?

If you're not the right owner for this issue, could you please remove yourself as soon as possible or help us find the right one?

If the issue is fixed or you can't reproduce it, please close the bug. If you've started working on a fix, please set the status to Started.

Thanks for your time! To disable nags, add the Disable-Nags label.

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

sky: Uh oh! This issue still open and hasn't been updated in the last 29 days. This is a serious vulnerability, and we want to ensure that there's progress. Could you please leave an update with the current status and any potential blockers?

If you're not the right owner for this issue, could you please remove yourself as soon as possible or help us find the right one?

If the issue is fixed or you can't reproduce it, please close the bug. If you've started working on a fix, please set the status to Started.

Thanks for your time! To disable nags, add the Disable-Nags label.

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

https://cs.chromium.org/chromium/src/chrome/browser/ui/views/bookmarks/bookmark_editor_view.cc?sq=package:chromium&dr=C&l=380

calls:
    url_tf_->SetText(chrome::FormatBookmarkURLForDisplay(url));

That, in turn, is commented thusly:
base::string16 FormatBookmarkURLForDisplay(const GURL& url) {
  // Because this gets re-parsed by FixupURL(), it's safe to omit the scheme
  // and trailing slash, and unescape most characters.  However, it's
  // important not to drop any username/password, or unescape anything that
  // changes the URL's meaning.
  return url_formatter::FormatUrl(
      url, url_formatter::kFormatUrlOmitAll &
               ~url_formatter::kFormatUrlOmitUsernamePassword,
      net::UnescapeRule::SPACES, nullptr, nullptr, nullptr);
}

Keeping the userinfo (username/password) is not compatible with omitting the scheme, because most schemes work like scheme://user:password@host/path and if you omit the scheme it becomes user:password@host, which, upon reinterpretation, is indistinguishable from scheme:user@host, because non-hierarchical schemes use ":" rather than "://" as the scheme delimiter.

To fix, we could change (url_formatter::kFormatUrlOmitAll & ~url_formatter::kFormatUrlOmitUsernamePassword) to kFormatUrlOmitTrailingSlashOnBareHostname, or we could more selectively do so iff the URL contains a userinfo component. I'm inclined to say that we should just always show the scheme, especially because today it's only omitted for HTTP (not HTTPS) and HTTPS is getting ever more common.

Thanks for taking on this elawrence@. 

https://codereview.chromium.org/2368593002

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

commit fa34e547d6ee25ea0692436ba7462ed0a0ef45f4
Author: elawrence <elawrence@chromium.org>
Date: Mon Oct 03 18:41:02 2016

Prevent interpretating userinfo as url scheme when editing bookmarks

Chrome's Edit Bookmark dialog formats urls for display such that a
url of http://javascript:scripttext@host.com is later converted to a
javascript url scheme, allowing persistence of a script injection
attack within the user's bookmarks.

This fix prevents such misinterpretations by always showing the
scheme when a userinfo component is present within the url.

BUG= 639126 

Review-Url: https://codereview.chromium.org/2368593002
Cr-Commit-Position: refs/heads/master@{#422467}

[modify] https://crrev.com/fa34e547d6ee25ea0692436ba7462ed0a0ef45f4/chrome/browser/ui/bookmarks/bookmark_utils.cc
[modify] https://crrev.com/fa34e547d6ee25ea0692436ba7462ed0a0ef45f4/chrome/browser/ui/bookmarks/bookmark_utils.h
[modify] https://crrev.com/fa34e547d6ee25ea0692436ba7462ed0a0ef45f4/chrome/browser/ui/cocoa/bookmarks/bookmark_editor_controller_unittest.mm
[modify] https://crrev.com/fa34e547d6ee25ea0692436ba7462ed0a0ef45f4/chrome/browser/ui/views/bookmarks/bookmark_editor_view_unittest.cc

Externally reported, so requesting merge to M-54

[Automated comment] Less than 2 weeks to go before stable on M54, manual review required.

SGTM, approved for merging into M54

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

commit 2775e31152857adc2bb9775b03212d1356541b4b
Author: Andrew R. Whalley <awhalley@chromium.org>
Date: Mon Oct 10 21:47:53 2016

[merge to m54] Prevent interpretating userinfo as url scheme when editing bookmarks

Chrome's Edit Bookmark dialog formats urls for display such that a
url of http://javascript:scripttext@host.com is later converted to a
javascript url scheme, allowing persistence of a script injection
attack within the user's bookmarks.

This fix prevents such misinterpretations by always showing the
scheme when a userinfo component is present within the url.

BUG= 639126 

Review-Url: https://codereview.chromium.org/2368593002
Cr-Commit-Position: refs/heads/master@{#422467}
(cherry picked from commit fa34e547d6ee25ea0692436ba7462ed0a0ef45f4)

Review URL: https://codereview.chromium.org/2411473002 .

Cr-Commit-Position: refs/branch-heads/2840@{#708}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/2775e31152857adc2bb9775b03212d1356541b4b/chrome/browser/ui/bookmarks/bookmark_utils.cc
[modify] https://crrev.com/2775e31152857adc2bb9775b03212d1356541b4b/chrome/browser/ui/bookmarks/bookmark_utils.h
[modify] https://crrev.com/2775e31152857adc2bb9775b03212d1356541b4b/chrome/browser/ui/cocoa/bookmarks/bookmark_editor_controller_unittest.mm
[modify] https://crrev.com/2775e31152857adc2bb9775b03212d1356541b4b/chrome/browser/ui/views/bookmarks/bookmark_editor_view_unittest.cc

Nice one, the panel awarded $500 for this bug.  They noted it's not really a UXSS, and needs a fairly uncommon combination of user interaction.  Thanks for the report!  A member of our finance team will be in touch shortly.

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

commit 2775e31152857adc2bb9775b03212d1356541b4b
Author: Andrew R. Whalley <awhalley@chromium.org>
Date: Mon Oct 10 21:47:53 2016

[merge to m54] Prevent interpretating userinfo as url scheme when editing bookmarks

Chrome's Edit Bookmark dialog formats urls for display such that a
url of http://javascript:scripttext@host.com is later converted to a
javascript url scheme, allowing persistence of a script injection
attack within the user's bookmarks.

This fix prevents such misinterpretations by always showing the
scheme when a userinfo component is present within the url.

BUG= 639126 

Review-Url: https://codereview.chromium.org/2368593002
Cr-Commit-Position: refs/heads/master@{#422467}
(cherry picked from commit fa34e547d6ee25ea0692436ba7462ed0a0ef45f4)

Review URL: https://codereview.chromium.org/2411473002 .

Cr-Commit-Position: refs/branch-heads/2840@{#708}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/2775e31152857adc2bb9775b03212d1356541b4b/chrome/browser/ui/bookmarks/bookmark_utils.cc
[modify] https://crrev.com/2775e31152857adc2bb9775b03212d1356541b4b/chrome/browser/ui/bookmarks/bookmark_utils.h
[modify] https://crrev.com/2775e31152857adc2bb9775b03212d1356541b4b/chrome/browser/ui/cocoa/bookmarks/bookmark_editor_controller_unittest.mm
[modify] https://crrev.com/2775e31152857adc2bb9775b03212d1356541b4b/chrome/browser/ui/views/bookmarks/bookmark_editor_view_unittest.cc

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