301069 - Bug 298934 (Dialog Origin Spoofing) not fixed on Mac

Status: RESOLVED FIXED

(Core :: Security, defect, P2)

Description

[Tom Hessman]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b4) Gecko/20050716 Firefox/1.0+
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b4) Gecko/20050716 Firefox/1.0+

Bug 298934 addresses SA15489, Dialog Origin Spoofing.  However, the fix from
that bug doesn't apply to Mac OS X, because sheets don't have a visible title or
title bar on that platform (so it doesn't display the source URL anywhere).  I
will attach a screenshot.

One possible way to fix this would be to include the URL after the dialog text,
in brackets, or some other formatting to make it distinct from the dialog text.

I verified this in Firefox 1.0.5 and the 2005-07-16 trunk build.  If possible,
this should probably be fixed in the upcoming 1.0.6, since it's already been
fixed on all other platforms...

Reproducible: Always

Comment 1

[Tom Hessman]

Attachment: Screenshot of dialog

Here is a screenshot showing the problem (lack of the originating URL in the
dialog).

Comment 2

[Tom Hessman]

Since the spoofing vulnerability was announced as fixed in Firefox 1.0.5, but
wasn't on the Mac, this should probably be fixed in 1.0.6.  Nominating.

Comment 3

[Daniel Veditz [:dveditz]]

There is a visual difference, though perhaps not one a user would consciously
notice. In a real prompt from the target page the sheet would show at the top.
In the spoof the sheet is attached to a tiny titlebar floating mid-page.

No need for the security flag since the original secunia announcement is public.

Comment 4

[Simon Fraser [no longer active]]

I think that FF should be changed to not use sheets for these dialogs. The
window title should be visible.

Comment 5

[Benjamin Smedberg]

I disagree. I think that we *should* be using sheets (tab-modal sheets, but
that's another bug), but we should put the site name in the content of the sheet
itself.

Comment 6

[Chase Phillips]

Per 1.0.6 meeting, minusing for aviary1.0.6, nominating for aviary1.0.7.

Comment 7

[Asaf Romano (gone)]

(In reply to comment #5)
> I disagree. I think that we *should* be using sheets (tab-modal sheets, but
> that's another bug), but we should put the site name in the content of the sheet
> itself.

Indeed, mac ordinary dialogs expose the dialog header inside the dialog and keep
the titlebar empty. see: http://tinyurl.com/bdozq 

Comment 8

[Asaf Romano (gone)]

Attachment: Expose dialog titles - v1

Comment 9

[Josh Aas]

Comment on attachment 190312 [details] [diff] [review]
Expose dialog titles - v1

-+  content/global/commonDialog.js	       (commonDialog.js)
-+  content/global/commonDialog.xul	       (commonDialog.xul)
+*+  content/global/commonDialog.js		(commonDialog.js)
+*+  content/global/commonDialog.xul		(commonDialog.xul)

Fix the spacing issue you introducted there, r=josh

Comment 10

[Asaf Romano (gone)]

Please file a separate bug for the suite, thanks.

Checking in toolkit/content/commonDialog.js;
/cvsroot/mozilla/toolkit/content/commonDialog.js,v  <--  commonDialog.js
new revision: 1.9; previous revision: 1.8
done
Checking in toolkit/content/commonDialog.xul;
/cvsroot/mozilla/toolkit/content/commonDialog.xul,v  <--  commonDialog.xul
new revision: 1.6; previous revision: 1.5
done
Checking in toolkit/content/jar.mn;
/cvsroot/mozilla/toolkit/content/jar.mn,v  <--  jar.mn
new revision: 1.17; previous revision: 1.16
done
Checking in toolkit/themes/pinstripe/global/formatting.css;
/cvsroot/mozilla/toolkit/themes/pinstripe/global/formatting.css,v  <-- 
formatting.css
new revision: 1.5; previous revision: 1.4
done

Comment 11

[Tom Hessman]

I think the way it looks with this fix is somewhat ugly...the body text doesn't
look right in it's non-bold form, and the URL may as well be in a title bar!  I
still think it would look better at the bottom instead of the top...the actual
message text should be bold, as before, and the URL/dialog title should be the
plain text, at the bottom instead of the top.  IMO, this would be more visually
appealing...

Comment 12

[Daniel Veditz [:dveditz]]

Comment on attachment 190312 [details] [diff] [review]
Expose dialog titles - v1

a=dveditz for drivers

Comment 13

[Asaf Romano (gone)]

Daniel, this patch does not affect the mozilla suite portion, see cooment 10.

Comment 14

[Asaf Romano (gone)]

fixed-aviary1.0.8:
Checking in toolkit/content/commonDialog.js;
/cvsroot/mozilla/toolkit/content/commonDialog.js,v  <--  commonDialog.js
new revision: 1.1.28.1.2.3; previous revision: 1.1.28.1.2.2
done
Checking in toolkit/content/commonDialog.xul;
/cvsroot/mozilla/toolkit/content/commonDialog.xul,v  <--  commonDialog.xul
new revision: 1.1.42.1; previous revision: 1.1
done
Checking in toolkit/content/jar.mn;
/cvsroot/mozilla/toolkit/content/jar.mn,v  <--  jar.mn
new revision: 1.6.8.5.2.1; previous revision: 1.6.8.5
done
Checking in toolkit/themes/pinstripe/global/formatting.css;
/cvsroot/mozilla/toolkit/themes/pinstripe/global/formatting.css,v  <--  formatting.css
new revision: 1.1.2.1.2.1; previous revision: 1.1.2.1
done

Comment 15

[Marcia Knous [:marcia]]

verified using Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.7.13) Gecko/20060215 Firefox/1.0.8. Using the test case on the secunia site, the dialog titles look fine. adding keyword.