49943 - World-readable e-mail messages left in /tmp

Status: VERIFIED FIXED

(MailNews Core :: Security, defect, P3)

Description

[Matt Siemens]

Any e-mail messages written in Mozilla Mail seem to cause two files to be
created in the /tmp directory (nsmail.eml and nsmail.html), both of which are
world readable.  These files contain the full contents of the e-mail message. 
As additional messages are written with Mozilla Mail, the files are incremented
as follows:

	nsmail-1.eml and nsmail-1.html
	nsmail-2.eml and nsmail-2.html
	.
	.
	.

Comment 1

[lchiang]

cc: to rhp and mscott and putterman.  Is mstoltz the right owner?

Comment 2

[Mitchell Stoltz (not reading bugmail)]

Yikes! This sort of thing shouldn't happen. I'm probably not the right owner, but 
please keep me in the loop. Reassigning to mscott.

Comment 3

[Scott MacGregor]

these messages are generated by mime when you go to send the message. They are
deleted after you send the message.

I don't think this is as big a security problem as it sounds. 4.x had this
behavior too.

If there's a code pathe where we aren't cleaning up after ourselves then that's
another matter.

Comment 4

[timeless]

Um, could we at least have this file 600 (u+rw g-rwx o-rwx)?
imo this file should be in ~/.mozilla/tmp/ ...
nominating nsbeta3, I don't know of a security/privacy keyword but i'd like 
this to be fixed (if possible)

Comment 5

[Mitchell Stoltz (not reading bugmail)]

Another thing which would make this safer would be to give the file a name which 
can't be determined by an attacker; something derived from a time value, or some 
other unguessable piece of internal state. This is essentially what the network 
cache does.

Comment 6

[scottputterman]

I just did a quick look at my temp directory and although not every mail I've 
sent is on there, I have about 60 .eml files as recent as 8/13 going back to 
earlier this year. I assume that means we aren't cleaning up in all cases.

Comment 7

[scottputterman]

marking nsbeta3+ per triage. We should investigate to see if we aren't cleaning 
up.

Comment 8

[timeless]

I found the equiv of this under w2k, although thanks to Terminal Services 
settings assuming i was using NTFS [which i wasn't] permissions would have been 
correctly set because it used a user specific temp dir.

OS: All,
kw: pp because on different platforms we do better/worse jobs of handling this 
problem.

Comment 9

[rhp (gone)]

I can handle this by changing permissions to the best of my ability on the 
platform.

- rhp

Comment 10

[lchiang]

second pass: - per mail triage since 4.x had the same behavior. Unfortunately, 
we're at a point where that is a criteria to minus a bug for nsbeta3.

Comment 11

[rhp (gone)]

Hi Folks,
I'd like to renominate this for consideration. This is a security hole that we 
should do what we can to prevent. Since this is a simple 2 line fix and it 
fixes a possible security issue, I ask for permission to check it in :-)

Here is the reviewed and tested patch:

Index: mozilla/mailnews/compose/src/nsMsgSend.cpp
===================================================================
RCS file: /cvsroot/mozilla/mailnews/compose/src/nsMsgSend.cpp,v
retrieving revision 1.203
diff -u -r1.203 nsMsgSend.cpp
--- nsMsgSend.cpp	2000/08/24 00:58:04	1.203
+++ nsMsgSend.cpp	2000/08/31 22:15:07
@@ -97,6 +97,8 @@
 #define PREF_MAIL_MESSAGE_WARNING_SIZE "mailnews.message_warning_size"
 #define PREF_MAIL_COLLECT_EMAIL_ADDRESS_OUTGOING 
"mail.collect_email_address_outgoing"
 
+enum  { kDefaultMode = (PR_WRONLY | PR_CREATE_FILE | PR_TRUNCATE) };
+
 #ifdef XP_MAC
 #include "xp.h"                 // mac only 
 #include "errors.h"
@@ -539,7 +541,7 @@
 		if (!mHTMLFileSpec)
 			goto FAILMEM;
 
-		nsOutputFileStream tempfile(*mHTMLFileSpec);
+		nsOutputFileStream tempfile(*mHTMLFileSpec, kDefaultMode, 
00600);
 		if (! tempfile.is_open()) 
     {
 			status = NS_MSG_UNABLE_TO_OPEN_TMP_FILE;
@@ -612,7 +614,7 @@
 	if (! mTempFileSpec)
 		goto FAILMEM;
 
-  mOutputFile = new nsOutputFileStream(*mTempFileSpec);
+  mOutputFile = new nsOutputFileStream(*mTempFileSpec, kDefaultMode, 00600);
 	if (! mOutputFile->is_open()) 
   {
 		status = NS_MSG_UNABLE_TO_OPEN_TMP_FILE;


Thanks!

- rhp

Comment 12

[lchiang]

Ok, I'll mark nsbeta3+.  Hurry and check it in to get this bug off our list :-)

Comment 13

[rhp (gone)]

The simple fix to change permissions to only allow the user to read the file is 
in the tree. We shouldn't leak tmp files in normal situations, but crashes, 
etc... are not normal situations :-)

- rhp

Comment 14

[varada]

verifying this - seems to still leave .eml and html files temp directories.

Comment 15

[Navin Gupta]

nsmail*.eml does not have correct permissions.

Comment 16

[rhp (gone)]

Ok, I see the very easy fix for this. Didn't catch all of the places files were 
opened, just the main one. 

Will try to mark as rtm and see what we get.

- rhp

Comment 17

[rhp (gone)]

Ok, here is the fix:

Index: mozilla/mailnews/compose/src/nsMsgAttachmentHandler.cpp
===================================================================
RCS file: /cvsroot/mozilla/mailnews/compose/src/nsMsgAttachmentHandler.cpp,v
retrieving revision 1.54
diff -u -r1.54 nsMsgAttachmentHandler.cpp
--- nsMsgAttachmentHandler.cpp	2000/09/13 23:53:13	1.54
+++ nsMsgAttachmentHandler.cpp	2000/10/05 18:57:21
@@ -466,7 +466,7 @@
         rv = NS_ERROR_FAILURE;
         goto done;
     }
-    mOutFile = new nsOutputFileStream(*mFileSpec, PR_WRONLY | PR_CREATE_FILE);
+    mOutFile = new nsOutputFileStream(*mFileSpec, PR_WRONLY | PR_CREATE_FILE, 
00600);
     if (!mOutFile)
     {
         rv =  NS_MSG_UNABLE_TO_OPEN_TMP_FILE;
@@ -542,7 +542,7 @@
   if (! mFileSpec )
   	return (NS_ERROR_FAILURE);
 
-  mOutFile = new nsOutputFileStream(*mFileSpec, PR_WRONLY | PR_CREATE_FILE);
+  mOutFile = new nsOutputFileStream(*mFileSpec, PR_WRONLY | PR_CREATE_FILE, 
00600);
   if (!mOutFile)
   {
     delete mFileSpec;
Index: mozilla/mailnews/compose/src/nsMsgSendLater.cpp
===================================================================
RCS file: /cvsroot/mozilla/mailnews/compose/src/nsMsgSendLater.cpp,v
retrieving revision 1.50
diff -u -r1.50 nsMsgSendLater.cpp
--- nsMsgSendLater.cpp	2000/09/13 23:53:14	1.50
+++ nsMsgSendLater.cpp	2000/10/05 18:57:25
@@ -1050,7 +1050,7 @@
       // and write the appropriate subset of the headers out. 
       m_inhead = PR_FALSE;
 
-			mOutFile = new nsOutputFileStream(*mTempFileSpec, 
PR_WRONLY | PR_CREATE_FILE);
+			mOutFile = new nsOutputFileStream(*mTempFileSpec, 
PR_WRONLY | PR_CREATE_FILE, 00600);
       if ( (!mOutFile) || (!mOutFile->is_open()) )
         return NS_MSG_ERROR_WRITING_FILE;
 

It is VERY simple and safe and fixes what I think is a security issue.

- rhp

Comment 18

[rhp (gone)]

First review by jefft is completed. Getting super review now.

- rhp

Comment 19

[rhp (gone)]

Ok, super-review = mscott

Mother may I :-)

Comment 20

[scottputterman]

adding rtm+ so PDT can review. It would be good to not allow others remote
access to old sent message files.

Comment 21

[Phil Peterson]

PDT marking [rtm++]. Has anyone done an audit of other user profile data to make
sure it isn't world-readable?

Comment 22

[rhp (gone)]

Ok, I just checked in code that should address this issue.

- rhp

Comment 23

[Navin Gupta]

verifying this bug

Comment 24

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

as rhp and mscott know, we've since found that there are are at least two
reproducable cases where world readble e-mail message are left in /tmp

posting to news
sending a draft or template

see bug #58580.

the fix for this bug did fix one case, but it didn't fix them all.

I'm going to mark this verified, but know that your can still leave
world-readable email files in /tmp

(the fix will be landing on the trunk soon.)

the workaround is to set your umask to 077.