Closed Bug 265668 Opened 20 years ago Closed 20 years ago

Live bookmarks can have javascript: and data: URLs

Categories

(Firefox :: Bookmarks & History, defect)

x86
Windows ME
defect
Not set
critical

Tracking

()

RESOLVED FIXED

People

(Reporter: mromarkhan, Assigned: vlad)

References

()

Details

(Keywords: fixed-aviary1.0, Whiteboard: [sg:fix] security)

Attachments

(2 files, 2 obsolete files)

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.
Attached file Testcase - an rss feed (deleted) —
I put one up at http://members.rogers.com/mromarkhan/RssFeed.html So you can click the lil RSS icon at the bottom right
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: blocking-aviary1.0?
Flags: blocking-aviary1.0? → blocking-aviary1.0+
Attached patch 265668-js-urls-in-livemark-feeds-0.patch (obsolete) (deleted) — Splinter Review
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.)
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++..)
Attachment #163055 - Attachment is obsolete: true
Comment on attachment 163063 [details] [diff] [review] 265668-js-urls-in-livemark-feeds-1.patch r+sr=jst
Attachment #163063 - Flags: superreview+
Attachment #163063 - Flags: review+
Fixed; thanks for catching this, Omar!
Status: NEW → RESOLVED
Closed: 20 years ago
Keywords: fixed-aviary1.0
Resolution: --- → FIXED
Comment on attachment 163063 [details] [diff] [review] 265668-js-urls-in-livemark-feeds-1.patch a=asa for aviary checkin.
Attachment #163063 - Flags: approval-aviary+
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.
Severity: minor → critical
Attached patch 265668-priv-urls-in-bmgr-2.patch (obsolete) (deleted) — Splinter Review
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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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 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
Attachment #163088 - Flags: superreview+
Attachment #163088 - Flags: review+
(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?
(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.
Status: REOPENED → RESOLVED
Closed: 20 years ago20 years ago
Resolution: --- → FIXED
What protocols are allowed / disallowed in Live Bookmarks with the patch that was checked in?
Summary: Javascript urls can execute with chrome privilege → Live bookmarks can have javascript: and data: URLs
Whiteboard: security
Whiteboard: security → [sg:fix] security
*** Bug 268820 has been marked as a duplicate of this bug. ***
Blocks: 248511
Security Advisories published, clearing confidential flag
Group: security
Blocks: sbb?
Blocks: sbb+
No longer blocks: sbb?
See also bug 312108.
sorry for bugspam, long-overdue mass reassign of ancient QA contact bugs, filter on "beltznerLovesGoats" to get rid of this mass change
QA Contact: mconnor → bookmarks
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: