History can fill FF3 addressbar suggestions (awesomebar) with unwanted bookmarklets that look almost like real pages but execute in the current page's scope.

VERIFIED FIXED in Firefox 3 beta4

Status

()

P1
major
VERIFIED FIXED
11 years ago
9 years ago

People

(Reporter: hansschmucker, Assigned: Mardak)

Tracking

Trunk
Firefox 3 beta4
Points:
---
Dependency tree / graph
Bug Flags:
blocking-firefox3 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:moderate?], URL)

Attachments

(1 attachment)

(Reporter)

Description

11 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b4pre) Gecko/2008021404 Minefield/3.0b4pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b4pre) Gecko/2008021404 Minefield/3.0b4pre

javascript: urls that return a html page when visited currently produce a history entry just like normal urls... while this may be desired for the classical history view, it's very dangerous that these urls end up in the address bar suggestion list as these entries can be made to look just like the real page, including FavIcon, Title and a pseudo url that only differs from the real url in that http:// is replaced with javascript:'//. However opening the javascript: url will execute it in the scope of the currently viewed page, thereby exposing all sensitive data on the page to the javascript code. I think javascript: urls should only be viewed in the suggestion list when they actually are bookmarklets and the user has made a conscious effort to add them, not when he just happened to come by one (which he may not even has noticed, the javascript: url in the sample is opened by an onclick handler and redirects to the correct page).

A simple filter for the suggestion box to not include history entries that start with javascript: should already be sufficient.

Reproducible: Always

Steps to Reproduce:
1.Visit http://tapper-ware.net/devel/js/historypoisoning/
2.Click on the link (http://www.news.com/8301-13578_3-9872464-38.html)
3.Open a new tab and start typing "news" into the addressbar
Actual Results:  
The javascript url appears in the suggestion list as "[CNet Icon] news.com BitTorrent firms: Comc(...)" with url "javascript:'//www.news.com/8301-13578_3-9872464-38.html______(...)"

Expected Results:  
The real news.com page should be the only listed suggestion as the javascript url is not a location that can safely be visited.

The favicon doesn't always work... I'm still trying to figure out when it does and when it doesn't but while it may be relevant to the practical severity of this bug, it doesn't change anything for the underlying flaw.
(Reporter)

Updated

11 years ago
OS: Windows XP → All
Hardware: PC → All
Version: unspecified → Trunk
(Reporter)

Updated

11 years ago
Summary: History can fill FF3 addressbar suggestions with unwanted bookmarklets that look almost like real pages. → History can fill FF3 addressbar suggestions with unwanted bookmarklets that look almost like real pages but execute in the current page's scope.

Updated

11 years ago
Flags: blocking-firefox3?
Whiteboard: [sg:moderate?]
(Reporter)

Comment 1

11 years ago
No takers? I think this is really quite critical since a malicious page can redirect thru a bookmarklet for any number of times thereby increasing its rank in the suggestion list... or it could make subtle changes to the bookmarklet each time it redirects so that all entries in the suggestion list consist of this bookmarklet... combine that with the situation that almost anybody activates a suggestion entry by accident every now and then because the mouse cursor often jumps around a pixel or two and you have a semi-automatic cross-domain attack.
Seems like a pretty straightforward way to exploit the new awesomebar feature (I mean, even I understand it). How many javascript URIs are revisited such that we need to include them in the awesomebar results? Easier just to filter all those out?

(Can we get a confirmation?)
Component: History → Places
Flags: blocking-firefox3? → blocking-firefox3+
QA Contact: history → places
(Reporter)

Comment 3

11 years ago
Well, I can confirm it (obviously) :)

There are a couple of good solutions IMHO:
1. Don't list javascript: history entries in the awesomebar
2. For history javascript: urls (NOT for bookmarklets) create a blank page first and execute the javascript url in that page's scope.
3. Save the page url along with the javascript url when adding such an entry to the history... when the user opens such an url, open the page url first, and execute the javascript: url onload.

#1 and #2 seem more "right", but #3 might be the most practical if we assume that a number of pages might generate anonymous popups that the user might want to revisit (not the case so far in my oppinion).

#2 has the problem that we'll probably populate the awesome bar with a lot of non-working javascript urls.

So my preference would be #1 by far, followed by #3 (which might be non-trivial to implement as we'd need a referer field in places which I don't think exists so far) and #2 if we really want to get on peoples nerves...
(Reporter)

Comment 4

11 years ago
P.S. I'd appreciate it if you didn't kill the JS url icons in the process as these are at least in my opinion incredibly useful, as they allow for bookmarklets with an icon in a way that doesn't need a special API or Addon and doesn't break anything.

Example:
http://tapper-ware.net/devel/js/bookmarklets/
(Don't try to drag the two buttons: Click them, then drag the tab title of the appearing description page to the bookmarks bar)
(Assignee)

Comment 5

11 years ago
Would it be okay to only show javascript: URIs if the user explicitly types out "javascript:" as the first part of the query? Not for "javascript" or "java", "alert javascript:", etc. Would it be safe to assume that if the user is typing out javascript:, s/he should know the consequences?

dietrich, gavin: Should we only filter out these results in the UI but still let the backend provide those results if some other autocomplete consumer wants those results?
(Reporter)

Comment 6

11 years ago
Sounds good to me...

Comment 7

11 years ago
Mardak's idea sounds great.
(Assignee)

Comment 8

11 years ago
Posted patch v1Splinter Review
Only allow searches that start with "javascript:" to find javascript URIs. + testcase! :)
Assignee: nobody → edilee
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #304911 - Flags: review?(dietrich)
(Reporter)

Updated

11 years ago
Summary: History can fill FF3 addressbar suggestions with unwanted bookmarklets that look almost like real pages but execute in the current page's scope. → History can fill FF3 addressbar suggestions (awesomebar) with unwanted bookmarklets that look almost like real pages but execute in the current page's scope.
Comment on attachment 304911 [details] [diff] [review]
v1

what's the use-case for browser.urlbar.filter.javascript = false?
(Assignee)

Comment 10

11 years ago
If someone wants to do multi word matches without needing to type out javascript:? Or someone else using nsNavHistory that does want javascript uris to always show for some other app? I figured we shouldn't need to set policy for everyone.
(Assignee)

Comment 11

11 years ago
With the pref set to false, an advanced user could use a saved javascript: bookmark by typing just the name of the bookmark instead of needing to type "javascript: bookmark"
(Reporter)

Comment 12

11 years ago
I think we should treat bookmarklets separately... and also with a separate pref.

0:Show matches in title or url (without requiring javascript:)
1:Show matches in title (without requiring javascript:)
2:Show matches in title or url (requiring javascript:)
3:Show matches in title (requiring javascript:)
4:Show no matches

browser.urlbar.filter.javascript.bookmarks: Default 1
browser.urlbar.filter.javascript.history: Default 3

That way users can have fine grained control over what results they want, but medium-advanced users get a setting they can work with right away as bookmarklets get listed by title, while average users (as long as they don't accidentally add bookmarklets) have nothing to worry about...
(Assignee)

Comment 13

11 years ago
Well, already providing a pref to turn it on and off is providing quite a bit more functionality than now. Dietrich, do we want a pref at all?
(Reporter)

Comment 14

11 years ago
Definately... but is there a reason not to add two prefs? I mean, the about:config list has reached the point where you can't navigate it without the search years ago . And if the prefs are only read at startup, it shouldn't really affect performance either, right?
Comment on attachment 304911 [details] [diff] [review]
v1

this is fine for now, better to start with this constraint and enhance as appropriate afterwards. the suggestions in comment #12 should be filed as a followup bug.
Attachment #304911 - Flags: review?(dietrich) → review+
Are data: URLs affected by this bug? Usually when javascript: is mentioned, data: URLs have the same problem.
(Assignee)

Comment 17

11 years ago
Checking in browser/app/profile/firefox.js;
/cvsroot/mozilla/browser/app/profile/firefox.js,v  <--  firefox.js
new revision: 1.283; previous revision: 1.282
done
Checking in toolkit/components/places/src/nsNavHistory.cpp;
/cvsroot/mozilla/toolkit/components/places/src/nsNavHistory.cpp,v  <--  nsNavHistory.cpp
new revision: 1.259; previous revision: 1.258
done
Checking in toolkit/components/places/src/nsNavHistory.h;
/cvsroot/mozilla/toolkit/components/places/src/nsNavHistory.h,v  <--  nsNavHistory.h
new revision: 1.141; previous revision: 1.140
done
Checking in toolkit/components/places/src/nsNavHistoryAutoComplete.cpp;
/cvsroot/mozilla/toolkit/components/places/src/nsNavHistoryAutoComplete.cpp,v  <--  nsNavHistoryAutoComplete.cpp
new revision: 1.43; previous revision: 1.42
done
RCS file: /cvsroot/mozilla/toolkit/components/places/tests/unit/test_417798.js,v
done
Checking in toolkit/components/places/tests/unit/test_417798.js;
/cvsroot/mozilla/toolkit/components/places/tests/unit/test_417798.js,v  <--  test_417798.js
initial revision: 1.1
done
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Flags: in-testsuite+
Priority: -- → P1
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3 beta4
(Assignee)

Updated

11 years ago
Blocks: 419237
(Assignee)

Updated

11 years ago
Blocks: 405926

Updated

11 years ago
Duplicate of this bug: 416904

Comment 19

11 years ago
*** VERIFIED

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9b5pre) Gecko/2008030607 Minefield/3.0b5pre

-Mike
verified with:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b5) Gecko/2008032606 Firefox/3.0b5
and
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-GB; rv:1.9b5) Gecko/2008032604 Firefox/3.0b5
Status: RESOLVED → VERIFIED
(Assignee)

Updated

11 years ago
Blocks: 448033
Duplicate of this bug: 452411
Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h".

In Thunderbird 3.0b, you do that as follows:
Tools | Message Filters
Make sure the correct account is selected. Click "New"
Conditions: Body   contains   places-to-b-and-h
Change the action to "Delete Message".
Select "Manually Run" from the dropdown at the top.
Click OK.

Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter.

Gerv
Component: Places → Bookmarks & History
QA Contact: places → bookmarks
You need to log in before you can comment on or make changes to this bug.