Last Comment Bug 251297 - files in /tmp world readable (email attachments, helper app docs)
: files in /tmp world readable (email attachments, helper app docs)
: fixed-aviary1.0, fixed1.7.5
Product: Core Graveyard
Classification: Graveyard
Component: File Handling (show other bugs)
: Trunk
: x86 Linux
: -- normal (vote)
: ---
Assigned To: Daniel Veditz [:dveditz]
: Hixie (not reading bugmail)
: 264372 292069 (view as bug list)
Depends on:
Blocks: 248511
  Show dependency treegraph
Reported: 2004-07-13 21:47 PDT by Daniel
Modified: 2016-06-22 12:16 PDT (History)
15 users (show)
mscott: blocking‑aviary1.0+
See Also:
QA Whiteboard:
Iteration: ---
Points: ---

Only change permissions if saving the file (1.22 KB, patch)
2004-10-22 06:29 PDT, Daniel Veditz [:dveditz]
cbiesinger: review+
bzbarsky: superreview+
chofmann: approval‑aviary+
chofmann: approval1.7.5+
Details | Diff | Splinter Review

Description Daniel 2004-07-13 21:47:49 PDT
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7) Gecko/20040626 Firefox/0.9.1
Build Identifier: Thunderbird 0.7.1

I was comparing treatment of attachments opened directly from emails on
different platforms. I discovered that Linux builds save attachments in /tmp
with world readable rights. This doesn't seem like a good thing. Couldn't
someone else logged onto the same machine read your attachments? It seems like
the user profile is a better place for an attachment cache. Or at the very least
the files could be saved with more restrictive permissions.

Sorry if there's something I'm missing and this bug report is invalid.

Reproducible: Always
Steps to Reproduce:
1. Find an email with an attachment
2 [review]. Open the attachment from within the email by double clicking on it

Actual Results:  
The attachment was saved to /tmp with world readable rights

Expected Results:  
Save it in a more secure location (user profile directory) and/or with more
secure rights (only owner readable).
Comment 1 Daniel Veditz [:dveditz] 2004-09-13 12:51:49 PDT
Also mentioned in bug 258578 comment 5 -- confirming.

This is not a good thing, fix for release. Mozilla mail surely does the same,
should I file another bug or can this one cover both?
Comment 2 Scott MacGregor 2004-09-28 16:14:10 PDT
dveditz, currently the uriloader opens attachments (and external brower clicks
likd PDF files, etc) using the following permission bits in the temp directory:

mTempFile->CreateUnique(nsIFile::NORMAL_FILE_TYPE, 0600);

is 0600 the right value to use or should we be using something different?


for where the permissions get set on this temporary file.

Comment 3 Scott MacGregor 2004-09-30 11:31:31 PDT

shows that the utf8 converter is indeed returning an unescaped url even though
we passed in an escaped one.

Also, Dan is there a reason why this is listed as a security senstive bug? This
seems like a normal bug fix to me....just curious.
Comment 4 Daniel Veditz [:dveditz] 2004-09-30 11:54:39 PDT
Was comment 3 intended for bug 243504 instead? That one is not security
sensitive. World-readable files containing potentially sensitive mail is a
security issue.
Comment 5 Scott MacGregor 2004-09-30 11:57:01 PDT
heh yup that comment was for the other bug :) sorry about that. 

what do you think about comment #2? What permissions should the uriloader be
setting on the temp files it creates for attachments and for browser external
helper apps?
Comment 6 Daniel Veditz [:dveditz] 2004-09-30 12:23:30 PDT
Regarding comment 2: 0600 seems like it would do the right thing, but I just
confirmed on brendan's machine that temp attachments *are* coming up world
readable. Either that call's not the one actually creating the attachment or
something below it has gone wonky. Someone's going to have to step through it in
a debugger.
Comment 7 Scott MacGregor 2004-09-30 14:16:21 PDT
When I step through this on windows I see the temp file getting created in the
temp directory with a call to CreateUnique with a value of 0600.

The resulting file permissions:


which to me says: read, write, execute for the owner of the file.

I don't see any UNIX specific code around this file creating code in the
uriloader. Very strange. 
Comment 8 Daniel Veditz [:dveditz] 2004-10-21 17:19:30 PDT
*** Bug 264372 has been marked as a duplicate of this bug. ***
Comment 9 Christian :Biesinger (don't email me, ping me on IRC) 2004-10-21 17:34:03 PDT
-> file handling where this code is.
Comment 10 Daniel Veditz [:dveditz] 2004-10-21 17:53:34 PDT
files were world readable: bug 107088
Then they weren't readable enough: bug 124307
now they're too readable: <you are here>

As dbaron says in bug 264372 comment 5 MoveFile() isn't the right place to call
FixFilePermissions, it should be up in ExecuteDesiredAction

but watch out for regressing part of bug 213921. I don't know what other actions
might fall into that "else", make sure to call FixFilePermissions --only-- if
the action is saveToDisk.
Comment 11 Daniel Veditz [:dveditz] 2004-10-22 06:29:34 PDT
Created attachment 163020 [details] [diff] [review]
Only change permissions if saving the file
Comment 12 Boris Zbarsky [:bz] (still a bit busy) 2004-10-22 07:48:50 PDT
Comment on attachment 163020 [details] [diff] [review]
Only change permissions if saving the file

Looks fine to me if biesi's OK with it.
Comment 13 chris hofmann 2004-10-22 08:06:16 PDT
Comment on attachment 163020 [details] [diff] [review]
Only change permissions if saving the file

a=chofmann for the branches
Comment 14 Christian :Biesinger (don't email me, ping me on IRC) 2004-10-22 09:31:55 PDT
Comment on attachment 163020 [details] [diff] [review]
Only change permissions if saving the file

r=biesi; a bit more context would've been nice

thanks for making this patch.
Comment 15 Daniel Veditz [:dveditz] 2004-10-22 14:34:16 PDT
Checked in to aviary and 1.7 branches. Trunk is waiting on smoketest blockers
Comment 16 sairuh (rarely reading bugmail) 2004-10-24 17:04:29 PDT
vrfy'd fixed with firefox 2004102409-0.9+ and tbird 2004102305-0.8 on linux fc2.
tested files and attachment opened via helper apps (respectively).
Comment 17 sairuh (rarely reading bugmail) 2004-10-24 17:05:36 PDT
forgot to add: while firefox and thunderbird were running, the files opened in
/tmp only had 600 privs. and once I had quit firefox/tbird, the files were
removed (as expected).
Comment 18 Daniel Veditz [:dveditz] 2004-10-25 00:23:45 PDT
Fixed on trunk
Comment 19 Daniel Veditz [:dveditz] 2004-10-25 03:57:00 PDT
Posted on public security lists, removing confidential flag.
Comment 20 Daniel Veditz [:dveditz] 2004-11-03 20:41:59 PST
adding secunia advisory information to catch duplicate filings: SA12956
Comment 21 Juha-Matti Laurio 2004-11-10 07:30:40 PST
(In reply to comment #20)
> adding secunia advisory information to catch duplicate filings: SA12956

In this advisory Firefox 1.0 is now marked as patched.

Update to version 1.0."
Comment 22 Jo Hermans 2005-09-02 16:24:17 PDT
*** Bug 292069 has been marked as a duplicate of this bug. ***

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