Last Comment Bug 265668 - Live bookmarks can have javascript: and data: URLs
: Live bookmarks can have javascript: and data: URLs
Status: RESOLVED FIXED
[sg:fix] security
: fixed-aviary1.0
Product: Firefox
Classification: Client Software
Component: Bookmarks & History (show other bugs)
: unspecified
: x86 Windows ME
: -- critical (vote)
: ---
Assigned To: Vladimir Vukicevic [:vlad] [:vladv]
:
Mentors:
http://members.rogers.com/mromarkhan/...
: 268820 (view as bug list)
Depends on:
Blocks: 248511 sbb+
  Show dependency treegraph
 
Reported: 2004-10-22 12:33 PDT by Omar Khan
Modified: 2006-08-27 07:26 PDT (History)
7 users (show)
asa: blocking‑aviary1.0+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Testcase - an rss feed (626 bytes, text/xml)
2004-10-22 12:36 PDT, Omar Khan
no flags Details
265668-js-urls-in-livemark-feeds-0.patch (3.71 KB, patch)
2004-10-22 12:57 PDT, Vladimir Vukicevic [:vlad] [:vladv]
no flags Details | Diff | Review
265668-js-urls-in-livemark-feeds-1.patch (5.66 KB, patch)
2004-10-22 13:16 PDT, Vladimir Vukicevic [:vlad] [:vladv]
jst: review+
jst: superreview+
asa: approval‑aviary+
Details | Diff | Review
265668-priv-urls-in-bmgr-2.patch (2.76 KB, patch)
2004-10-22 16:25 PDT, Vladimir Vukicevic [:vlad] [:vladv]
jst: review+
jst: superreview+
Details | Diff | Review

Description Omar Khan 2004-10-22 12:33:00 PDT
User-Agent:       Mozilla/5.0 (Windows; U; Win 9x 4.90; rv:1.7.3) Gecko/20041001 Firefox/0.10.1
Build Identifier: Mozilla/5.0 (Windows; U; Win 9x 4.90; rv:1.7.3) Gecko/20041001 Firefox/0.10.1

This requires that
the current url is
chrome.
Say for example
chrome://browser/content/bookmarks/bookmarksManager.xul.
And the user clicks the bookmarklet in this
context.

<a href="javascript: var m_prefs =
Components.classes[&quot;@mozilla.org/preferences-service;1&quot;].getService(Components.interfaces.nsIPrefBranch);
m_prefs.setCharPref(&quot;browser.startup.homepage&quot;,&quot;http://visuallinkindicator.mozdev.org/&quot;);">Link</a>
- change home page

Can be a problem with livemarks.


Reproducible: Always
Steps to Reproduce:
1. Bookmark link
2. Enter chrome://browser/content/bookmarks/bookmarksManager.xul
3. Click bookmark link

Actual Results:  
Home page changed or javascript with chrome
privilege executed.

Expected Results:  
Eventhough it is a nice feature,
I think it should deny.
Comment 1 Omar Khan 2004-10-22 12:36:35 PDT
Created attachment 163051 [details]
Testcase - an rss feed

I put one up at
http://members.rogers.com/mromarkhan/RssFeed.html
So you can click the lil RSS icon at the
bottom right
Comment 2 Vladimir Vukicevic [:vlad] [:vladv] 2004-10-22 12:57:44 PDT
Created attachment 163055 [details] [diff] [review]
265668-js-urls-in-livemark-feeds-0.patch

Skip javascript and chrome URLs in live bookmark feeds; note that this makes
the test feed show up as "live bookmark failed to load", which is what happens
when there are no valid items to load.	(Should say Empty, but too late for
that.)
Comment 3 Vladimir Vukicevic [:vlad] [:vladv] 2004-10-22 13:16:29 PDT
Created attachment 163063 [details] [diff] [review]
265668-js-urls-in-livemark-feeds-1.patch

Changes based on jst's feedback.  Use nsIScriptSecurityManager's CheckLoadURI
method for this instead of doing our own homegrown version.  (We need to add
deps on caps, xpconnect, and js for this though, even to call it from C++..)
Comment 4 Johnny Stenback (:jst, jst@mozilla.com) 2004-10-22 13:19:37 PDT
Comment on attachment 163063 [details] [diff] [review]
265668-js-urls-in-livemark-feeds-1.patch

r+sr=jst
Comment 5 Vladimir Vukicevic [:vlad] [:vladv] 2004-10-22 13:22:16 PDT
Fixed; thanks for catching this, Omar!
Comment 6 Asa Dotzler [:asa] 2004-10-22 13:26:52 PDT
Comment on attachment 163063 [details] [diff] [review]
265668-js-urls-in-livemark-feeds-1.patch

a=asa for aviary checkin.
Comment 7 Ben Bucksch (:BenB) 2004-10-22 14:41:02 PDT
So, you are handling this only for live feeds? If so, this bug still exists, if
the user bookmarks a malice URL manually (either by "bookmark this link" or
maybe trickily putting a Javascript URL in urlbar and still having an
interesting page showing up.
Comment 8 Vladimir Vukicevic [:vlad] [:vladv] 2004-10-22 16:25:29 PDT
Created attachment 163088 [details] [diff] [review]
265668-priv-urls-in-bmgr-2.patch

Yeah, ok, Ben's right.. this can be pretty bad.  This is a gross hack; however,
every bookmarks open funnels in to here, so it's hard to decide where we're
actually at.  Any alternative suggestions welcome.
Comment 9 Daniel Veditz [:dveditz] 2004-10-22 16:35:09 PDT
This is one of the oldest chrome-privilege-giveaway bugs we had in Seamonkey,
we've got to stop making the same mistakes all over again!
Comment 10 Johnny Stenback (:jst, jst@mozilla.com) 2004-10-22 16:53:16 PDT
Comment on attachment 163088 [details] [diff] [review]
265668-priv-urls-in-bmgr-2.patch

>+  // don't allow loading javascript/data urls

Add /chrome there...

r+sr=jst
Comment 11 Christian :Biesinger (don't email me, ping me on IRC) 2004-10-22 17:04:17 PDT
(In reply to comment #8)
> Created an attachment (id=163088)
> 265668-priv-urls-in-bmgr-2.patch

wouldn't calling CheckLoadURI here require slightly less hardcoding of various
stuff?
Comment 12 Vladimir Vukicevic [:vlad] [:vladv] 2004-10-22 17:13:50 PDT
(In reply to comment #9)
> This is one of the oldest chrome-privilege-giveaway bugs we had in Seamonkey,
> we've got to stop making the same mistakes all over again!

So, we actually don't have the bug that I thought we did (and that I was trying
to fix).  I misunderstood the original bug report to mean that bookmarklets
executed from the bookmarks manager were running with chrome privs; but that
isn't the case.  Stripping js/etc. from live bookmark feeds is still valid, so
that'll stay; however, the example chrome URL threw me off.

The issue here is that if the user navigates to a chrome: URI in a browser
window and then runs a bad bookmarklet, that bookmarklet executes with chrome
privs.  This I'm inclined to say is not a bug then, as it requires:

1) the user manually navigates to a chrome: URI
2) the user executes a bookmarklet that does bad stuff

Returning this back to fixed; ignore the second patch.
Comment 13 Jesse Ruderman 2004-10-22 18:03:51 PDT
What protocols are allowed / disallowed in Live Bookmarks with the patch that
was checked in?
Comment 14 timeless 2004-11-10 13:23:31 PST
*** Bug 268820 has been marked as a duplicate of this bug. ***
Comment 15 Daniel Veditz [:dveditz] 2005-01-24 13:44:05 PST
Security Advisories published, clearing confidential flag
Comment 16 Jesse Ruderman 2005-10-11 15:56:05 PDT
See also bug 312108.
Comment 17 Mike Connor [:mconnor] 2006-08-27 07:26:48 PDT
sorry for bugspam, long-overdue mass reassign of ancient QA contact bugs, filter on "beltznerLovesGoats" to get rid of this mass change

Note You need to log in before you can comment on or make changes to this bug.