Closed Bug 251297 Opened 20 years ago Closed 20 years ago

files in /tmp world readable (email attachments, helper app docs)

Categories

(Core Graveyard :: File Handling, defect)

x86
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: friedturkey, Assigned: dveditz)

References

Details

(Keywords: fixed-aviary1.0, fixed1.7.5, Whiteboard: [sg:fix])

Attachments

(1 file)

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).
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?
Group: security
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: blocking-aviary1.0?
Whiteboard: [sg:fix]
Flags: blocking-aviary1.0? → blocking-aviary1.0+
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?

See:

http://lxr.mozilla.org/mozilla/source/uriloader/exthandler/nsExternalHelperAppService.cpp#1335

for where the permissions get set on this temporary file.

Status: NEW → ASSIGNED
http://lxr.mozilla.org/seamonkey/source/intl/uconv/src/nsUTF8ConverterService.cpp#118

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.
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.
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?
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.
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:

-rwx------+

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. 
*** Bug 264372 has been marked as a duplicate of this bug. ***
-> file handling where this code is.
Assignee: mscott → file-handling
Status: ASSIGNED → NEW
Component: General → File Handling
Product: Thunderbird → Browser
QA Contact: ian
Version: unspecified → Trunk
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.
Assignee: file-handling → dveditz
Component: File Handling → Accessibility APIs
Version: Trunk → 1.0 Branch
Summary: attachments opened from emails saved to /tmp with world readable rights → files in /tmp world readable (email attachments, helper app docs)
Component: Accessibility APIs → File Handling
Version: 1.0 Branch → Trunk
Attachment #163020 - Flags: superreview?(bzbarsky)
Attachment #163020 - Flags: review?(cbiesinger)
Attachment #163020 - Flags: approval1.7.x?
Attachment #163020 - Flags: approval-aviary?
Whiteboard: [sg:fix] → [sg:fix] need reviews
Comment on attachment 163020 [details] [diff] [review]
Only change permissions if saving the file

Looks fine to me if biesi's OK with it.
Attachment #163020 - Flags: superreview?(bzbarsky) → superreview+
Comment on attachment 163020 [details] [diff] [review]
Only change permissions if saving the file

a=chofmann for the branches
Attachment #163020 - Flags: approval1.7.x?
Attachment #163020 - Flags: approval1.7.x+
Attachment #163020 - Flags: approval-aviary?
Attachment #163020 - Flags: approval-aviary+
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.
Attachment #163020 - Flags: review?(cbiesinger) → review+
Whiteboard: [sg:fix] need reviews → [sg:fix] has patch ready to land
Checked in to aviary and 1.7 branches. Trunk is waiting on smoketest blockers
Whiteboard: [sg:fix] has patch ready to land → [sg:fix] needs trunk landing
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).
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).
Fixed on trunk
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Whiteboard: [sg:fix] needs trunk landing → [sg:fix]
Posted on public security lists, removing confidential flag.
Group: security
adding secunia advisory information to catch duplicate filings: SA12956
http://secunia.com/advisories/12956/
(In reply to comment #20)
> adding secunia advisory information to catch duplicate filings: SA12956
> http://secunia.com/advisories/12956/

In this advisory Firefox 1.0 is now marked as patched.
"Solution:

Firefox:
Update to version 1.0."
Blocks: 248511
*** Bug 292069 has been marked as a duplicate of this bug. ***
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.