76463 - formpost files should use 600, not 666, for permissions

Status: VERIFIED FIXED

(Core :: DOM: Core & HTML, defect, P1)

Description

[Jedi/Sector One]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (X11; U; Linux 2.4.3-ac9 i686; en-US; 0.8.1) Gecko/20010412
BuildID:    2001041214

When a file is uploaded through a form, Mozilla creates a /tmp/formpost file (or
formpost-xxx to avoid overwriting) .
This file is created with the 666^~umask perms. On most distros, the standard
umask is 022, therefore, /tmp/formpost* files are world readable. As they can
hold private content, this is an insecure behavior.

Reproducible: Always
Steps to Reproduce:
1.Upload a file through a form
2.Read /tmp/formpost

Actual Results:  Any user can read the posted data.

Expected Results:  Permissions should be enforced to 600

Comment 1

[Boris Zbarsky [:bzbarsky]]

This is technically a dup of bug 15320.  But that bug has recently gotten
futured, so the correct solution (not creating temp files) is not going to
happen anytime soon.

confirming this bug as a request to just change the file permissions.  In that
form, nominating for beta1 and mozilla0.9.1 as a major security hole.

Comment 2

[rods (gone)]

reassigning, known issue, probably a dup

Comment 3

[Eric Pollmann]

Unfutured the other bug.  Marking this a dup.  Time to clean this up...

*** This bug has been marked as a duplicate of 15320 ***

Comment 4

[Vladimir Ermakov]

ok, verifying

Comment 5

[Boris Zbarsky [:bzbarsky]]

Reopening since this is a much more serious issue than bug 15320 and that bug is
nowhere close to being fixed.

Comment 6

[Darin Fisher]

taking 

Comment 7

[Boris Zbarsky [:bzbarsky]]

Attachment: Proposed patch

Comment 8

[Darin Fisher]

this patch looks good, except it might be really nice to take this time to
switch over to using nsIFile instead of nsFileSpec (since you are switching from
nsIFileSpec to nsFileSpec... might as well do the right thing and switch to
nsIFile).

Comment 9

[Boris Zbarsky [:bzbarsky]]

I was trying to, actually, but I see no way to do the equivalent of MakeUnique()
with nsIFile.  I suppose I could roll my own....  would that be better than
using nsFileSpec for this functionality?

Comment 10

[Darin Fisher]

-> bz, pointed him to nsIFile::CreateUnique... he's going to work on a new patch
that uses nsIFile :)

Comment 11

[Boris Zbarsky [:bzbarsky]]

Attachment: Patch using nsIFile

CreateUnique is a bastard child that doesn't do what you'd expect it to... But
if you treat it right, it _can_ do the right thing.  :)

Comment 12

[Boris Zbarsky [:bzbarsky]]

Alexandru, would you review?

Comment 13

[Darin Fisher]

Comment on attachment 59981 [details] [diff] [review]
Patch using nsIFile

sr=darin

BTW it's valid to write NS_ADDREF(*aMultipartDataFile = tempDir)... NS_ADDREF
actually corresponds to an inline function :)

Comment 14

[Alexandru Savulov]

Comment on attachment 59981 [details] [diff] [review]
Patch using nsIFile

r= alexsavulov

OPTIONAL:
could we use everywhere nsIFile and get rid of nsIFileSpec usage?

Comment 15

[Boris Zbarsky [:bzbarsky]]

Yes, but that's a separate bug... :)

Comment 16

[Boris Zbarsky [:bzbarsky]]

checked in

Comment 17

[Vladimir Ermakov]

no more temp files on linux.. verifying