An attacker can steal a copy of a file from a Mozilla mail user's hard drive if - the attacker knows the name and location of the file on the user's hard drive, and - the attacker can get the victim to reply to a message as HTML or as HTML+plain. When a user replies to a message containing a literal <img src="file:///...">, Mozilla attaches that file to the message (if the file exists). If the file doesn't exist, Mozilla leaves the progress window up but never finishes sending the message. The user can cancel out of the Sending Messages progress window. If the src given is a directory, then while the progress window is up, I get an alert "Unable to open the temporary file %.200s. Check your 'Temporary Directory' setting and try again" and the message is not sent. If the user chooses to send the message as plain text only, the attacker does not recieve a copy of the file. (Bug 88110 causes the message to be sent as multipart/related rather than text/plain in this case, but that's not a security risk.) I wasn't able to figure out a way for the attacker to make the dialog not appear or change the default from "plain text only", but I'm not very familiar with this part of Mozilla mail. Here's what I pasted into judge (a Netscape mail server) on port 25 using telnet: -- begin paste -- mail from:jesse rcpt to:jesse data Subject: foop Content-Type: text/html foo <img src="file:///C:/WINNT/Profiles/Administrator/Desktop/nss.txt" alt=bar> baz . -- end paste -- When I replied to this message, a copy of nss.txt was attached.

cc-ing related people.

Jean-Francois, can you look into this?

security related. Mark PDT+ for now to get on PDT radar.

Cc cmanske since I think this is his area.

Accepting and looking at it...

I have a fix but that cause a little regression: parts are not send anymore...

Please tell me that was the wrong patch. I have no idea what the patch is trying to do.

Can somebody in editor team review this patch. Thanks

The patch is the correct one. This is the function that build a list of embedded objects for mail editor. Mail compose will use this list to determine the parts that need to be attached to the message. I have modified it to skip over blockquote node of type "cite" which will prevent use to reattach any object received. Oops, I should block only object with file URL! Let me change it...

OK, as far as I can see, the patch changes nsHTMLEditor::GetEmbeddedObjects() so that it does not return objects inside of a <blockquote>. I don't think this is the correct fix for the bug. nsHTMLEditor::GetEmbeddedObjects() is not mail specific, and is used elsewhere (e.g. by the code that saves a page with images to disk), and will be used by publishing. If you want to persue this approach, you should, I think, not change this method, but have the caller filter out elements on the result based on whether they are nested inside of a blockquote. Also, does this really address the problem? Is it always the case that "dangerous" images will be inside the blockquote?

ok, I should maybe address the problem right after the quoting (and before the user start typing). I can parse the nodes (I can use nsHTMLEditor::GetEmbeddedObjects)and either kill any local file url or set an attribute on the node that I can check during the send to avoid to attach the part.

It's possible for users to edit the quoted text, and add new images in there. Your current fix will stop these images being sent. Doing the work at quoting time would be better, but why not filter file:// URLs out of the text that gets quoted, before inserting into the editor (i.e. before quoting)?

In fact, if I totally suppress the file url, I wonder if I will kill some legitimate cases (imagine a file url on a corporate file server). Therefore, I think the best solution would be to just add an attribute to the node that we can filter during the send. The recipient will get the message, with the URL but without a copy of the file.

*** Bug 88079 has been marked as a duplicate of this bug. ***

Here is the deal. Right after inserting the original message body into editor, I retreive the embedded objects (nsHTMLEditor::GetEmbeddedObjects) and check if they are using a file url. If it's the case, I add an attribute to the node. During the send, I look if the embedded object as this new attribute set and if it's, I just ignore the part. The original link is not affected. Varada, can you review?

Also, I fixed couple potential crashes I found while debugging this patch.

Coding style comments: + nsString objURL; Better to use nsAutoString. + domElement->SetAttribute(NS_ConvertASCIItoUCS2("DoNotSend"), NS_ConvertASCIItoUCS2("true")); NS_LITERAL_STRING ? Also, I think we should use a -moz_ attribute name, so it's obvious that this is some internal thing. + if (NS_SUCCEEDED(domElement-> HasAttribute(NS_ConvertASCIItoUCS2("DoNotSend"), &hasAttribute))) NS_LITERAL_STRING Does this patch do the right thing with <file://> URLs in signatures/vCards?

Are the crashes that you fixed the cause of bug 88079?

That's possible, I need to check...

I'll address simon remarks. >Does this patch do the right thing with <file://> URLs in signatures/vCards? This patch does not touch the user's signature or the user's vcard.

This patch does not fix bug 88079

in patch v3, I have addressed sfraser concerns and I have also found some missing { in the if/else if chain.

I haven't tried the patch, but it looks like it only blocks file:/// URLs from being attached. I didn't think of this when I first reported the bug, but if I mail myself a message with <img src="http://bugzilla.mozilla.org/show_bug.cgi?id=88124"> and reply to it, the contents of this (netscape-confidential!) bug are attached to the reply message. The only things that should be attached are: - attachments the user creates (including images the user inserts) - attachments and images that came with the original message - images and other objects embedded by (not linked to by!) the sender's signature or vcard.

Right, this is blocking only file urls. I'll see if I can block more...

Why not just keep a list of images the user has explicitly attached, and then attach those when sending, rather than scanning the message body in the first place? I don't know if we automatically attach images from files which the user has attached, if so, then just add stuff to the list recursively, and cull duplicates at the end. Do we send file urls when the mail is forwarded as an attachment? My solution would take care of that, as well.

the patch v4 will block any embedded object that wasn't a part of the original message. I have tested it for imap, pop3 and news.

oops! ignore the changes in nsMsgCompUtils.cpp, it's for another bug...

varada, jess, can you review it?

+ rv = NS_NewURI(getter_AddRefs(uri), (const char *)nsAutoCString(objURL)); Implicit downsizing of a UCS2 string to ASCII is nasty. This would be better written using CopyUCS2toASCII(). More comments soon.

+ nsIMsgMessageService * msgService = nsnull; + rv = GetMessageServiceFromURI(mQuoteURI, &msgService); Does this leak? Or is GetMessageServiceFromURI() "special"? + PRBool hasAttribute; + if (NS_SUCCEEDED(domElement->HasAttribute(NS_LITERAL_STRING("moz-do-not- send"), &hasAttribute))) + if (hasAttribute) + continue; This fails if somehow the element has moz-do-not-send="false". You should check the value of the attribute too.

I have addressed all simon's comment. Good catch about msgService, I forget to call ReleaseMessageServiceFromURI().

You guys need to write a stack-based class that does GetMessageServiceFromURI()/ ReleaseMessageServiceFromURI(). sr=sfraser on the patch.

r=varada

Fix checked in the trunk and the 0.9.2 branch

Jesse, can you see if this is fixed using the latest branch build since I am unable to reproduce the problem? If you prefer just state more detailed steps on how I can reproduce. Assigning this bug to nbaca since sheelar will be out for a couple days.

Trunk build 2001-07-13-04: WinMe, Linux RH 6.2, Mac 9.04 verifiedtrunk. I tried the following scenario: a. In the Browser go to "bugzilla.mozilla.org" b. Enter a bug number so you view the details in bugzilla (i.e. 88124) c. Select File|Send Page and address it to yourself d. In Mail, retrieve the message 1. Select the message, select the Reply button and send the message back to yourself. 2. Select the message, select the Reply button, Insert an image and send the message back to yourself (this is to check for regressions). e. Using 4.x view the message source for d1 (with moz/6.x current builds, if the message is too large then you cannot view the entire page source, known bug) f. Using 4.x view the message source for d2 Actual Results: 1. e: The message source does Not show the "mozilla-banner.gif" reference followed by hex characters. 2. f: The message has a reference to the gif file followed by hex characters as expected. I hope this makes sense.

Branch build: 07-13-06: Win Branch build: 07-13-04: Linux RH 6.2 Branch build: 07-13-03: Mac 8.6 Verified Fixed. Esther and Sheela performed the following scenario which nicely displays the problem. Using an older build such as PR1: 1. User1 telnets a message to User2 which includes the path and a file name on User2's system: --begin paste-- mail from:esther rcpt to:esther data Subject: foop Content-Type: text/html foo <img src="file:///C:/graphics/graphic1.jpg" alt=bar> baz . --end paste-- Note: User2 never sent the graphic file to User1. User1 just happens to know that this file exists on User2's system. 2. User2 receives the message a. User2 replies to the message b. User2 replies to the same message but also inserts a different graphic file (graphic2.jpg) 3. User1 retrieves message 2a. 4. User1 retrieves message 2b. Actual Results: 3. User1 receives the message and now sees the graphic file (graphic1.jpg), bad. 4. User1 receives the message and sees both graphic files (graphic1.jpg and graphic2.jpg), bad. With the build from today, 7/13, the problem no longer occurs: Actual Results: 3. User1 never receives the graphic file (graphic1.jpg), as expected. 4. User1 never receives graphic1.jpg but does receive graphic2.jpg, as expected.

Removing Netscape-Confidential