Last Comment Bug 164842 - Improper plugin tmp dir name on Linux
: Improper plugin tmp dir name on Linux
Status: RESOLVED FIXED
[PL2:NA]
:
Product: Core
Classification: Components
Component: Plug-ins (show other bugs)
: Trunk
: x86 Linux
: P3 normal with 1 vote (vote)
: mozilla1.5alpha
Assigned To: Nobody; OK to take it and work on it
:
: Benjamin Smedberg [:bsmedberg]
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2002-08-27 05:41 PDT by tenthumbs
Modified: 2013-04-17 02:27 PDT (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments

Description tenthumbs 2002-08-27 05:41:20 PDT
In nsPluginHostImpl.cpp at line 186 there is

#define kPluginTmpDirName NS_LITERAL_CSTRING("plugtmp")

and it's used at around line 2040. On Linux mozilla creates a /tmp/plugtmp.
That's wrong for a multi-user OS. You need a unique name for every user.

Also mozilla tries to give this directory 0777 permissions. That's a huge
security/privacy hole particularly since the code says https download are stored
there.
Comment 1 Mitch 2002-08-27 08:14:01 PDT
we have this dir on our multi user environment here with
10+ users on the same machine. The /tmp/plugtmp dir exists
and is 0755 (masked probably by the users umask). Doesn't
seem to affect anything since it's probably silently 
ignored if mkdir() fails for another user, and also it's
completely empty - no https stuff there. 

Maybe it's just an artifact and is just dead code. Maybe
a candidate for code cleanup operations ?
Comment 2 rubydoo123 2002-08-27 09:49:44 PDT
assigning to Anthony for verification
Comment 3 serge (gone) 2002-08-27 10:11:59 PDT
IMHO directory with 0777 mode is fine, the files suppose to have 0600 mode.
it's the same in case of security concern
if we'll create /tmp/plugintmp/<user with 0600 mode>/file_owned_by_user
or /tmp/plugintmp/file_owned_by_user_with_0600_mode
Comment 4 anthonyd 2002-08-27 13:25:48 PDT
after talking with serge he says we at least have to change the file permissions
to 0600 for this bug, and we also need input from mstoltz our security guru
about this issue.

<ping> mstoltz
Comment 5 tenthumbs 2002-08-27 13:47:29 PDT
You need 0700 for a directory and 0600 for files in that directory. If a 
random user has r+x permission on a directory then he may be able to 
hard link to a file there.

I know the code's been there for quite a while but I've only seen the 
directory recently. Did something change?
Comment 6 serge (gone) 2002-08-27 14:00:01 PDT
the code's been in since 2001-11-06, when fix for bug 104859 was checked in.
Comment 7 tenthumbs 2002-08-27 14:05:01 PDT
Then something else changed. Mozilla never created /tmp/plugtmp back 
then. I pay attention to this sort of stuff.
Comment 8 Andrew Schultz 2002-08-27 21:30:35 PDT
if a directory has 0777 permissions, any user can write to the directory and any
user can delete files from the directory, regarless of the ownership or 
permissions of the files themselves

currently, the directory needs 0777 permissions because it is not unique (per user)
Comment 9 Mitchell Stoltz (not reading bugmail) 2002-09-03 17:04:26 PDT
Yes, this seems kind of bad - if this directory holds temporary data from
plugins then it should probably be accessible only to that user. Is it feasible
to create a new temp dir for each user, with the most restrictive permissions
possible? (0700 for the dir and 0600 for the files sounds good.) I'm no Unix
wizard, but maybe there's a shortcut call for creating per-user temp directories?
Comment 10 anthonyd 2002-09-26 15:07:47 PDT
Okay, I guess I was hoping someone would tell (and I am not a unix guy at all)
what I need to do here to fix this.  please?
Comment 11 anthonyd 2002-09-26 15:13:34 PDT
I guess to be mroe specific, could a unix guru tell me which solution mentioned
here is the correct or most correct one, or do I need another solution entirely?
Comment 12 Andrew Schultz 2002-09-26 16:13:40 PDT
I think comment 3a is probably the best solution:

/tmp/plugintmp/<user with 0600 mode>/file_owned_by_user

except that should be 0700, right?

/tmp/plugin_user/file_owned_by_user
would also work.  again, "plugin_user" should be mode 0700, read/write/execute,
but only for that user.
Comment 13 tenthumbs 2002-09-27 06:01:22 PDT
Each mozilla process needs to use unique names. You can't share.

If mozilla uses /tmp/plugindir/<anything> then /tmp/plugindir will be owned
by precisely one user and that user can probably delete /tmp/plugindir and
everything beneath it. At the very least that user can rename the directory.
What will mozilla do if the paths it knows about are suddenly invalid? This
is not just one user shooting himself in the foot. All active mozillas
belonging to other users will be affected.

Also, using a shared name means that it is possible for the directory to
exist for extended periods. Many systems will periodically reap the temp
dirs. You want to avoid giving such jobs the chance to delete every
mozilla's files.

I make no claims to be a unix guru but I do know a few things.

There are many unix horror stories about what can befall the unwary when
creating temp files. Most of them are true. There are many nasty surprises
possible. Luckily mozilla wants to create a directory which is easier
because directory semantics are more restrictive than regular files.

A few platforms offer a mkdtemp function which does exactly what you want.
Unfortunately it's not widely available, even within the same platform; e.g.
Redhat 7 has it but RH6 does not. That means that you have to roll your own
and that means using some variation of looping over creating a unique name,
checking if it already exists, and creating the directory. That's usually
tempnam + mkdir. There are lots of not-so-unique tempnam's out there so
there are warnings about using it as well as any other such functions. See
the man pages.

It may be better to do it yourself. You need to do three things:
1) choose a parent directory. If you want a system temp one then it's common
to use the TMPDIR environment variable first (provided you're not a suid
process), then the P_tmpdir from stdio.h, then /tmp.
2) create a reasonably unique name. That shouldn't be too difficult for
mozilla.
3) try to create a directory in the parent dir with the unique name.
If something goes wrong loop over the steps for some reasonable number of
times.

You really should get together with the bug 124307 people who have similar
needs. There are also bugs about mozilla not supporting TMPDIR. They all
should be addressed the same way.

This may be harder to do than it should be in mozilla because mozilla wants
to use nspr and nspr offers only a limited subset of the system functions
you need.

If you want to see a working model look at lynx. It does exactly the same
thing and it's been around a while so most of the rough edges have been
smoothed out.

BTW, note that 4.x uses nscomm40_<username> which is a start but 4.x can get
confused if that name already exists but is owned by another user.
Comment 14 rubydoo123 2002-12-06 14:48:23 PST
handing over to serge and putting into 1.3 beta
Comment 15 tenthumbs 2002-12-06 15:18:56 PST
BTW, look at bug 181146.
Comment 16 Peter Lubczynski 2003-03-27 16:45:19 PST
-->peterl
Comment 17 Jan Kasprzak 2009-02-20 06:11:19 PST
Any updates for this bug after no comments for ~6 years? Even the QA Contact of this bug is a non-existent address. This bug probably has security implications, but still it has been ignored for such a long time.

I think the proper solution would be to use mkstemp("/var/tmp/mozplug.XXXXXX") for each downloaded file, or mkdtemp("/var/tmp/mozplug.XXXXXX") per session.
Comment 18 Georg Fritzsche [:gfritzsche] 2013-04-17 02:27:20 PDT
We have had files stored into unique temp directories with mode 0700 for a while now (see nsPluginHost::GetPluginTempDir()).

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