Last Comment Bug 298934 - (sa15489) Replace "[Javascript Application]" in content-originating sheets with something more useful (SA15489)
(sa15489)
: Replace "[Javascript Application]" in content-originating sheets with somethi...
Status: RESOLVED FIXED
[sg:fix]
: fixed-aviary1.0.5, fixed1.7.9
Product: Core
Classification: Components
Component: Security (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Daniel Veditz [:dveditz]
:
Mentors:
http://secunia.com/multiple_browsers_...
: 171129 (view as bug list)
Depends on: 386486
Blocks: 301069
  Show dependency treegraph
 
Reported: 2005-06-27 11:33 PDT by Simon Fraser
Modified: 2007-07-02 01:29 PDT (History)
17 users (show)
dveditz: blocking1.7.9+
dveditz: blocking‑aviary1.0.5+
dveditz: blocking1.8b3+
bob: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Use host as alert/confirm/prompt title (when known) (4.76 KB, patch)
2005-07-02 01:11 PDT, Daniel Veditz [:dveditz]
cbiesinger: review+
jst: superreview+
jaymoz: approval‑aviary1.0.5+
jaymoz: approval‑aviary1.0.5+
jaymoz: approval1.7.9+
jaymoz: approval1.7.9+
jaymoz: approval1.8b3+
jaymoz: approval1.8b3+
Details | Diff | Splinter Review
same -w (3.93 KB, patch)
2005-07-02 01:12 PDT, Daniel Veditz [:dveditz]
no flags Details | Diff | Splinter Review
just the host, not scheme://host[:port] (4.27 KB, patch)
2005-07-04 00:35 PDT, Daniel Veditz [:dveditz]
cbiesinger: review+
jst: superreview-
Details | Diff | Splinter Review
Trunk version of the first patch. (4.60 KB, patch)
2005-07-05 23:19 PDT, Johnny Stenback (:jst, jst@mozilla.com)
no flags Details | Diff | Splinter Review

Description Simon Fraser 2005-06-27 11:33:09 PDT
For security and usability reasons, we should replace the text "[Javascript
Application]" in sheets that are raised from content with something that shows
the URL of the requesting page, perhaps with the page title and/or favicon.

It should be clear that this request is content-generated, not
browser-generated. Perhaps the sheet should have a distinctive look?
Comment 1 Simon Fraser 2005-06-29 21:35:36 PDT
This is actually a core bug. "[Javascript Application]" comes from gecko as the
window title, and fixing this will involve API changes.
Comment 2 Mike Pinkerton (not reading bugmail) 2005-06-30 06:19:30 PDT
all / all.

This should prolly block 1.0.5 as there's discussion on the lists about it.
Comment 3 Daniel Veditz [:dveditz] 2005-06-30 10:09:35 PDT
I don't know if this will actually block, but we'd like it fixed.
Comment 4 Jaime Mitchell (use bugmail@jaimem.org.uk for email) 2005-06-30 10:19:18 PDT
*** Bug 171129 has been marked as a duplicate of this bug. ***
Comment 5 Daniel Veditz [:dveditz] 2005-07-02 01:11:46 PDT
Created attachment 188014 [details] [diff] [review]
Use host as alert/confirm/prompt title (when known)
Comment 6 Daniel Veditz [:dveditz] 2005-07-02 01:12:33 PDT
Created attachment 188015 [details] [diff] [review]
same -w
Comment 7 Simon Fraser 2005-07-02 12:02:02 PDT
Two things:
1. I'd much prefer we pass the host string down to the embedder explicitly,
   rather than buried in a "window title" string, so that embedders can do
   smart things with it. This would require changing APIs, I know.

2. Is this really necessary? Can't the code that displays the dialogs get the
   host from the location, using the nsIDOMWindow passed in?
Comment 8 Christian :Biesinger (don't email me, ping me on IRC) 2005-07-03 17:54:41 PDT
Comment on attachment 188015 [details] [diff] [review]
same -w

hm, I don't get this code. Why are you not just using the host, instead of
first removing user:pass and then using the prePath?

also:
+	       aOutTitle = NS_ConvertUTF8toUTF16(prepath);

should be:
CopyUTF8toUTF16(prepath, aOutTitle);
Comment 9 Daniel Veditz [:dveditz] 2005-07-04 00:28:29 PDT
I'm pretty sure we're not up for changing API's on the aviary/1.7 branches.
nsIPrompt is one of the most basic frozen interfaces we've got, there might be a
lot of reluctance to change it even on the trunk. I doubt the pain is worth the
gain, but if you can get the embedding folks to agree I've got no problem doing
it that way (on the trunk).

(In reply to comment #7)
> 2. Is this really necessary? Can't the code that displays the dialogs get the
>    host from the location, using the nsIDOMWindow passed in?

Not in the case of javascript: or data: urls. Unless we go to the principal the
location doesn't tell us anything more than today's current "JavaScript
Application". And given that loophole that's  exactly what the next spoofing
demonstration would use.

(In reply to comment #8)
> hm, I don't get this code. Why are you not just using the host, instead of
> first removing user:pass and then using the prePath?

I thought the scheme and port might be good to have: makes it look more like an
address than a title, and in a few rare cases a non-standard port might be good
to know. I also tried putting just the host in [] like the current text; it was
a bit ugly but we could do that as well. If no one thinks the scheme is useful
it's a whole lot easier just getting the host.

> should be:
> CopyUTF8toUTF16(prepath, aOutTitle);

thanks
Comment 10 Daniel Veditz [:dveditz] 2005-07-04 00:35:14 PDT
Created attachment 188176 [details] [diff] [review]
just the host, not scheme://host[:port]

Is this better?
Comment 11 Christian :Biesinger (don't email me, ping me on IRC) 2005-07-04 07:35:29 PDT
Comment on attachment 188014 [details] [diff] [review]
Use host as alert/confirm/prompt title (when known)

oh, I forgot that prePath includes the scheme. may be better to include it,
then.

When fixup is unavailable, should you maybe fallback to using the url
unmodified?

r=me with or without that, plus the CopyUTF8toUTF16 change.
Comment 12 Christian :Biesinger (don't email me, ping me on IRC) 2005-07-04 07:41:33 PDT
Comment on attachment 188176 [details] [diff] [review]
just the host, not scheme://host[:port]

(but if you want to go with this instead, feel free. r=biesi)
Comment 13 Johnny Stenback (:jst, jst@mozilla.com) 2005-07-05 12:58:00 PDT
Comment on attachment 188014 [details] [diff] [review]
Use host as alert/confirm/prompt title (when known)

sr=jst
Comment 14 Johnny Stenback (:jst, jst@mozilla.com) 2005-07-05 13:01:29 PDT
Comment on attachment 188176 [details] [diff] [review]
just the host, not scheme://host[:port]

The first patch in this bug (the one I just sr+'ed) I'm ok with, but this
version pretty much gives any spoofing site full control over the dialog title,
with the exception of being limited to the characters  that are valid in host
names. If we do this, we'd need to have some code to truncate long URLs to
prevent sites from fully controlling the dialig title... Maybe we need to
change our code so that our dialog code assumes everything before the first
space is a hostname and does the truncation based on the width of the dialog
etc, or a whole new API...?
Comment 15 Jay Patel [:jay] 2005-07-05 22:13:43 PDT
Comment on attachment 188014 [details] [diff] [review]
Use host as alert/confirm/prompt title (when known)

a=jay for the branches and trunk
Comment 16 Jay Patel [:jay] 2005-07-05 22:14:01 PDT
Comment on attachment 188014 [details] [diff] [review]
Use host as alert/confirm/prompt title (when known)

a=jay for the branches and trunk
Comment 17 Johnny Stenback (:jst, jst@mozilla.com) 2005-07-05 23:19:16 PDT
Created attachment 188403 [details] [diff] [review]
Trunk version of the first patch.
Comment 18 Christian :Biesinger (don't email me, ping me on IRC) 2005-07-06 04:45:40 PDT
(In reply to comment #14)
> If we do this, we'd need to have some code to truncate long URLs to
> prevent sites from fully controlling the dialig title...

Hm, but, don't both patches here have that problem?
Comment 19 Johnny Stenback (:jst, jst@mozilla.com) 2005-07-06 15:28:10 PDT
(In reply to comment #18)
> (In reply to comment #14)
> > If we do this, we'd need to have some code to truncate long URLs to
> > prevent sites from fully controlling the dialig title...
> 
> Hm, but, don't both patches here have that problem?

The first one much less so than the second one. With the first patch, the user
would at least see "http://...", whereas with the second one the user would just
see the beginning of the domain name the attacker site uses w/o any good way to
know that they're seeing the domain name.
Comment 20 Juha-Matti Laurio 2005-07-13 17:47:49 PDT
This is new MFSA2005-54 advisory;
http://www.mozilla.org/security/announce/mfsa2005-54.html now.
Comment 21 Juha-Matti Laurio 2005-07-18 19:05:04 PDT
There is instructions written in June at my webpages
http://www.networksecurity.fi/#topic29 handling this issue in major and many
other browsers. Btw: this covers almost all browsers, from IE-based only Maxthon
(MyIE2)is immune. We were first who has fix now =)
"- Move the JavaScript dialog box from covering the original URL address at
location bar. [and that little extra window without content]
- When typing sensitive information to a Web site password-type dialog boxes, be
sure that this site is a legitimate site.
NOTE: Using multiple domain suffixes may indicate a spoofing attempt.
- In Mozilla-based browsers: Those dialog boxes mentioned are titled as
[JavaScript Application] (Javascript-sovelma in Finnish etc.)."
This advice is now available because many users has localized 1.0.4 while
waiting their own-language 1.0.6.
Comment 22 Juha-Matti Laurio 2005-07-18 19:09:27 PDT
For some reason text

Replace "[Javascript Application]" in content-originating sheets with something
more useful (SA15489)

was in QA Contact: field which prevent updating (comment). Please someone with
rights enough add this again to this entry.
Comment 23 therube 2005-09-19 13:20:14 PDT
Could someone confirm whether this actually is, or should be fixed in Mozilla
1.7.10, 1.7.11, & 1.7.12RC.

Cause if so, & if I'm understanding the secunia report correctly, it does not
seem to be?

"Secunia"
http://forums.mozillazine.org/viewtopic.php?p=1753319
Comment 24 Jesse Ruderman 2006-04-20 18:59:08 PDT
See bug 301069 for the Mac version of this bug.

See bug 334893 for more discussion of the UI.

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