New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 82181 link

Starred by 15 users

Issue metadata

Status: Fixed
Owner: ----
Closed: Mar 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android
Pri: 3
Type: Feature

Restricted
  • Only users with EditIssue permission may comment.



Sign in to add a comment

Strip javascript: schema from pastes/drops to omnibox

Reported by chromium...@gmail.com, May 10 2011

Issue description

In cases which commonly imply that a user is being socially engineered to XSS themselves with a javascript: uri we should strip the schema from the omnibox text. Power users can then re-enter the schema if they choose to but a navigation will cause a harmless search rather than self xss.

Behavior for paste and drag drop would be as follows.

On paste we simply strip any leading "javascript:" from the pasted text before inserting into the omnibox.

On drop we strip leading "javascript:" and cancel navigation.
 

Comment 1 by suzhe@chromium.org, May 12 2011

I think we can fix this issue in chrome/browser/autocomplete/autocomplete_edit.cc in a platform independent way. E.g. for paste operation, in AutocompleteEdit::OnAfterPossibleChange() method, we can know if a text change is caused by a paste operation and can do whatever we want accordingly.
And I think we can do similar fix in AutocompleteEdit::CanPasteAndGo() and PasteAndGo() for drop operation.
We're already working on a change that does this in a platform-specific way that I'm more comfortable with.

Comment 3 by cdn@google.com, May 13 2011

suzhe: I have a patch for this that works for gtk too. Basically it just delays resetting the paste requested flag until OnAfterPossibleChange and modifies the text there. It's a bit hackish as it allows the control to actually have the bad content for some period of time but it seems like that will be the case regardless unless we handle access to the clipboard directly instead of allowing the control do it.
Peter: do you still want to get this landed for windows/mac in the meantime? There is a patch up which doesn't touch gtk for now if you want to take a look.

Comment 6 by cdn@chromium.org, May 24 2011

Status: Fixed
Landed r86525 which implements this for Windows and Mac. We can examine doing this for Linux again when the omnibox implementation stops using a native control. 
Labels: -Mstone-13 -OS-All Mstone-X OS-Linux
Owner: ----
Status: Available
Summary: [Linux] Strip javascript: schema from pastes/drops to omnibox
Let's leave this open and covering Linux, but M-X it.
Cc: dpranke@chromium.org
If I understand the security implication, I would think it should be impossible for a copied string to result in a runnable javascript: url.

However, as I noted in  issue 85232 , on Mac, it seems the omnibox strips whitespace after checking. Therefore, whitespace at the beginning or newlines will be stripped from the middle of the protocol, leaving a runnable javascript url in the omnibox. Is there any way to check for javascript: after trimming whitespace?

For example, it is possible to use a U+202F narrow space, which then gets trimmed. An attacker may be able to trick someone into copying the space, as in the line below.

 javascript:alert("hello world");
Yes, we're fixing that stuff

Comment 11 by delv...@gmail.com, Jun 24 2011

How often do people have "self XSS" issues?

And how is an Omnibox script different than a content script?  In other words, why allow a javascript: schema at all?

Personally this is a minor inconvenience...

Daniel

Comment 12 by fam....@live.nl, Jun 24 2011

"Why allow a javascript: schema at all"
Well, because it's easy. I often use it to ask people for their localization (for debugging purposes for AdBlock), by asking to paste javascript:alert(navigator.language) in the address bar.

Also, sometimes I want to test things out and I do this by pasting code in the address bar until I have a correct working code. Lately I wanted to know if src in document.querySelectorAll("[src]") was case sensitive. That was when I discovered this bug that it strips javascript: (yes, I think it's an annoying bug).

Comment 13 by sh...@chromium.org, Jun 24 2011

#12, the JavaScript console is probably more appropriate for the execution of random JavaScript.  For people who know what they're doing, it's an inconvenience, but, unfortunately, there are a lot of people who don't realize what they're doing when someone asks them to paste an URL into the omnibox.
I'd give a number on the scale of self-XSS attacks but I'm not sure that information is public.  Suffice to say it is a very large number.

If you can take further debate to the chromium-discuss mailing list that would be great.  Bugs are not a good place to debate design decisions.

Comment 15 by Deleted ...@, Jul 7 2011

How about an option to turn this garbage off so it's not manipulating/breaking my input?

if i am pasting code in the address bar with a javascript prefix i expect that to execute accordingly not manipulate the schema and submit the damn thing anyway as it completely changes the purpose of the script i sincerely doubt there is any case where the input is "javascript:someaddress.com" so why would you bother submitting? if anything you should be blocking or just not providing changes like this without an option to disable it
Labels: Restrict-AddIssueComment-Commit
This is not a place to debate this feature.  Please take your comments to the chromium-discuss mailing list.
Project Member

Comment 17 by bugdroid1@chromium.org, Nov 24 2011

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=111548

------------------------------------------------------------------------
r111548 | ncj674@motorola.com | Thu Nov 24 11:19:29 PST 2011

Changed paths:
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/gtk/omnibox/omnibox_view_gtk.cc?r1=111548&r2=111547&pathrev=111548
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/autocomplete/autocomplete_edit.h?r1=111548&r2=111547&pathrev=111548
 M http://src.chromium.org/viewvc/chrome/trunk/src/base/utf_string_conversion_utils.h?r1=111548&r2=111547&pathrev=111548

Strip invalid characters (line breaks, tabs), javascript:schemes from the copied text while pasting text, droping text and creating right click popup for omnibox.

BUG= 82181 ,  103703 .
TEST=Copy a string with line breaks "\n" or tabs "\t".
Then right click on omnibox.
Chromium should not trigger DCHECKS.

Review URL: http://codereview.chromium.org/8513002
------------------------------------------------------------------------
Status: Fixed
Should be fixed in r111548 AFAIK.  Haven't tested.
Project Member

Comment 19 by bugdroid1@chromium.org, Nov 24 2011

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=111558

------------------------------------------------------------------------
r111558 | rsleevi@chromium.org | Thu Nov 24 13:25:25 PST 2011

Changed paths:
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/gtk/omnibox/omnibox_view_gtk.cc?r1=111558&r2=111557&pathrev=111558
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/autocomplete/autocomplete_edit.h?r1=111558&r2=111557&pathrev=111558
 M http://src.chromium.org/viewvc/chrome/trunk/src/base/utf_string_conversion_utils.h?r1=111558&r2=111557&pathrev=111558

Revert 111548 - Broke OmniboxViewTest.AcceptKeywordBySpace on Linux

Strip invalid characters (line breaks, tabs), javascript:schemes from the copied text while pasting text, droping text and creating right click popup for omnibox.

BUG= 82181 ,  103703 .
TEST=Copy a string with line breaks "\n" or tabs "\t".
Then right click on omnibox.
Chromium should not trigger DCHECKS.

Review URL: http://codereview.chromium.org/8513002

TBR=ncj674@motorola.com
Review URL: http://codereview.chromium.org/8690006
------------------------------------------------------------------------
Status: Started
Project Member

Comment 21 by bugdroid1@chromium.org, Dec 4 2011

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=112923

------------------------------------------------------------------------
r112923 | ncj674@motorola.com | Sat Dec 03 19:14:54 PST 2011

Changed paths:
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/omnibox/omnibox_view_browsertest.cc?r1=112923&r2=112922&pathrev=112923
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/gtk/omnibox/omnibox_view_gtk.cc?r1=112923&r2=112922&pathrev=112923
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/autocomplete/autocomplete_edit.h?r1=112923&r2=112922&pathrev=112923
 M http://src.chromium.org/viewvc/chrome/trunk/src/base/utf_string_conversion_utils.h?r1=112923&r2=112922&pathrev=112923

Strip invalid characters (line breaks, tabs), javascript:schemes from the copied text while pasting text, droping text and creating right click popup for omnibox.

BUG= 82181 ,  103703 

TEST=Copy a string with line breaks "\n" or tabs "\t".
Then right click on omnibox.
Chromium should not trigger DCHECKS.

Review URL: http://codereview.chromium.org/8702002
------------------------------------------------------------------------

Comment 22 by sh...@chromium.org, Dec 10 2011

Cc: cdn@chromium.org
 Issue 107065  has been merged into this issue.
 Issue 113998  has been merged into this issue.

Comment 24 by dhw@chromium.org, Sep 11 2012

 Issue 142091  has been merged into this issue.

Comment 25 by dhw@chromium.org, Sep 11 2012

 Issue 147464  has been merged into this issue.
Labels: OS-Android OS-iOS
If we're doing platform-specific fixes for this bug, do we need any for iOS or Android?  Tagging this bug appropriately so it hits the radar of the right people.

Cc: rohitrao@chromium.org
Can you clarify what you mean by "platform-specific" fixes?  Have we made recent changes to the "javascript"-stripping code that are platform-specific, or is this bug trying to track the initial iOS/Android implementations of the stripping functionality?
If you look you, you can see that fixes to strip javascript: were all done in platform specific ways:
comment #21 fixed this in gtk files
comment #5 fixed this in mm and views_win files

I assume (not knowing how UIs are built on mobile) that there might be additional places the need to be fixed on other platforms.

Project Member

Comment 29 by bugdroid1@chromium.org, Mar 10 2013

Labels: -Area-UI -Feature-Omnibox Cr-UI-Browser-Omnibox Cr-UI

Comment 30 by laforge@google.com, Mar 13 2013

Labels: -Restrict-AddIssueComment-Commit Restrict-AddIssueComment-EditIssue
Labels: -OS-iOS
This was fixed for iOS in Issue 228348.
Project Member

Comment 32 by sheriffbot@chromium.org, Jun 29 2016

Labels: Hotlist-OpenBugWithCL
A change has landed for this issue, but it's been open for over 6 months. Please review and close it if applicable. If this issue should remain open, remove the "Hotlist-OpenBugWithCL" label. If no action is taken, it will be archived in 30 days.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Hotlist-OpenBugWithCL
Status: Available (was: Started)
Summary: [Linux & Android] Strip javascript: schema from pastes/drops to omnibox (was: [Linux] Strip javascript: schema from pastes/drops to omnibox)
Still not fixed on some platforms apparently.
Components: -UI
Labels: -Pri-2 Pri-3
Status: Fixed (was: Available)
Summary: Strip javascript: schema from pastes/drops to omnibox (was: [Linux & Android] Strip javascript: schema from pastes/drops to omnibox)
Just tried to repro on Linux and Android, the only two platforms previous unfixed.  Both now strip the scheme.

Sign in to add a comment