164842 - Improper plugin tmp dir name on Linux

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect, P3)

Description

[tenthumbs]

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]

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]

assigning to Anthony for verification

Comment 3

[serge (gone)]

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]

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]

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)]

the code's been in since 2001-11-06, when fix for bug 104859 was checked in.

Comment 7

[tenthumbs]

Then something else changed. Mozilla never created /tmp/plugtmp back 
then. I pay attention to this sort of stuff.

Comment 8

[Andrew Schultz]

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)]

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]

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]

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]

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]

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]

handing over to serge and putting into 1.3 beta

Comment 15

[tenthumbs]

BTW, look at bug 181146.

Comment 16

[Peter Lubczynski]

-->peterl

Comment 17

[Jan Kasprzak]

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]]

We have had files stored into unique temp directories with mode 0700 for a while now (see nsPluginHost::GetPluginTempDir()).