Last Comment Bug 372035 - Need to prevent inadvertent bookmarking of javascript: and data: URLs
: Need to prevent inadvertent bookmarking of javascript: and data: URLs
Status: RESOLVED FIXED
: fixed-seamonkey1.1.8
Product: SeaMonkey
Classification: Client Software
Component: Security (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: neil@parkwaycc.co.uk
:
Mentors:
Depends on: 371179
Blocks:
  Show dependency treegraph
 
Reported: 2007-02-27 15:54 PST by Chris Thomas (CTho) [formerly cst@andrew.cmu.edu cst@yecc.com]
Modified: 2009-12-20 08:00 PST (History)
9 users (show)
iann_bugzilla: blocking‑seamonkey1.1.8+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Fix Add Bookmark (1.20 KB, patch)
2007-04-23 05:29 PDT, neil@parkwaycc.co.uk
csthomas: review+
dveditz: superreview+
iann_bugzilla: approval‑seamonkey1.1.8+
Details | Diff | Review

Description Chris Thomas (CTho) [formerly cst@andrew.cmu.edu cst@yecc.com] 2007-02-27 15:54:23 PST
From firefox bug 371179 (camino is 372020)
> User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; pl; rv:1.8.1.1)
> Gecko/20061204 Firefox/2.0.0.1
> Build Identifier: 
> 
> It should be not possible to bookmark data: and other inline / special URL
> schemes, or, if this functionality is needed, Javascript execution context
> should be properly initialized. Otherwise, such a bookmark, when invoked, will
> be allowed to execute Javascript code in the context of a currently viewed
> resource (Google start page = stealing GMail authentication cookies, for
> example).
> 
> See this demonstration:
> 
> http://lcamtuf.coredump.cx/ffbook/
> 
> Reproducible: Always
Comment 1 Benoît 2007-02-27 17:30:58 PST
Assigning to me.
Comment 2 Serge Gautherie (:sgautherie) 2007-02-27 19:58:55 PST
[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a3pre) Gecko/2007022704 Minefield/3.0a3pre] (nightly) (W2Ksp4)

For the record,
I can reproduce the Ctrl+D behaviour with the FF testcase,
and the JS exploit which leads me to <http://lcamtuf.coredump.cx/ffbook/success.html>.


[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a3pre) Gecko/20070227 SeaMonkey/1.5a] (nightly) (W2Ksp4)

I can reproduce the Ctrl+D behaviour with the FF testcase,
and not the JS exploit which leads me to <http://lcamtuf.coredump.cx/ffbook/failure.html>. (Maybe related to my profile ?)

*(D.o.) Bug 371179, as I guess the SM patch will be based after the FF one.
Comment 3 Caio Tiago Oliveira (asrail) 2007-02-28 17:03:09 PST
It would be nice to know what caused the failure for you Serge.
With the latest SM trunk, on Linux, it's the same behavior as with FX.

Maybe google.com redirects you to google.co.uk?
With google.com.br here it fails.

Comment 4 Serge Gautherie (:sgautherie) 2007-02-28 22:05:28 PST
[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a3pre) Gecko/20070228 SeaMonkey/1.5a] (nightly) (W2Ksp4)

No redirection.
I tested again, with a new profile, and it succeeded.
...
I guess the |document.cookie| in <http://lcamtuf.coredump.cx/ffbook/attack.js> is "blocked" in my usual profile.
[I don't like JS "playing" with cookies.]

Sorry for my first report:
I didn't try this, because the try+catch masked the exception I get otherwise.
It looks like that demonstration does not explicitly account for secure browser/profile ;-/
Comment 5 Robert Kaiser (not working on stability any more) 2007-04-23 05:00:44 PDT
What's up here? No action for almost two months on something marked as a blocker? We would strictly be entering code freezes for 1.0.9 and 1.1.2 today, and if we want to hold off those releases for this bug, as the blocking+ flags imply, we badly need a solution. Not tomorrow, but yesterday or some weeks ago, actually!

So, what's the status on this?
Comment 6 neil@parkwaycc.co.uk 2007-04-23 05:29:21 PDT
Created attachment 262484 [details] [diff] [review]
Fix Add Bookmark

As for Groupmarks, they work by adding new tabs for the bookmarks (and optionally deleting the old tabs), so they always run on a safe context.
Comment 7 Chris Thomas (CTho) [formerly cst@andrew.cmu.edu cst@yecc.com] 2007-04-23 05:48:07 PDT
Comment on attachment 262484 [details] [diff] [review]
Fix Add Bookmark

looks good.
Comment 8 Benoît 2007-04-23 15:12:45 PDT
[14:08] <BenoitRen> KaiRo: I'm sorry. I'm sorry. I'm sorry. :(
[14:08] <BenoitRen> KaiRo: Read the bugmail. What happened is that I looked at the Firefox patch, got confused, investigated it for a while, and then forgot about it.
[14:08] <BenoitRen> ("Read the bugmail." is meant as paste tense, I didn't add a comment to the bug.)
Comment 9 Daniel Veditz [:dveditz] 2007-04-24 02:19:26 PDT
Comment on attachment 262484 [details] [diff] [review]
Fix Add Bookmark

sr=dveditz

I'm not sure users will know that a data: url is potentially dangerous even if you show them the dialog
Comment 10 Andrew Schultz 2007-05-05 08:46:31 PDT
Does this still need to land on the branch(es)?
Comment 11 Serge Gautherie (:sgautherie) 2007-05-05 09:34:48 PDT
(In reply to comment #10)
> Does this still need to land on the branch(es)?

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8.0.12pre) Gecko/20070504 SeaMonkey/1.0.9] (nightly) (W2Ksp4)
[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8.1.4pre) Gecko/20070505 SeaMonkey/1.1.2] (nightly) (W2Ksp4)
[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a5pre) Gecko/20070504 SeaMonkey/1.5a] (nightly) (W2Ksp4)

Bug still there.

***

Well :-/
Actually, this patch has not been checked in anywhere yet.

NB:
</xpfe/components/bookmarks/resources/bookmarks.js>
was recently moved to
</suite/common/bookmarks/bookmarks.js>.
Comment 12 neil@parkwaycc.co.uk 2008-01-15 03:00:01 PST
Fix checked in to the trunk.
Comment 13 Andrew Schultz 2008-01-17 22:16:34 PST
Should we also be triggering a dialog for "Bookmark this link" and "Bookmark this page" from the context menu?
Comment 14 Ian Neal 2008-01-25 16:17:31 PST
a=me for SM1.1.8
Comment 15 neil@parkwaycc.co.uk 2008-01-25 16:28:03 PST
Fix checked in to the branch.

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