624995 - Does not set filesystem permissions correctly for saved files

Status: RESOLVED FIXED

(Core Graveyard :: File Handling, defect, P1)

Description

[Shuo Han]

User-Agent:       Mozilla/5.0 (Macintosh; Intel Mac OS X 10.5; rv:2.0b8) Gecko/20100101 Firefox/4.0b8
Build Identifier: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.5; rv:2.0b8) Gecko/20100101 Firefox/4.0b8

For every file saved from Firefox, the file is given the permission of "rw-------" (i.e. "600"). This makes the file not readable by other users, both locally and/or after being copied to another UNIX-like machine. The desired behavior is "rw-r--r--" (i.e. "644").

Reproducible: Always

Steps to Reproduce:
1. Save any file from Firefox
2. Check the file's permission by typing "ls -l <file_name>" in the Terminal window

Actual Results:  
The file has the permission of "rw-------".

Expected Results:  
The file should have the permission of "rw-r--r--".

Comment 1

[Matthias Versen [:Matti]]

what are the folder permissions of that folder where you save the file ?

Comment 2

[Shuo Han]

The folder permission is "rwxr-xr-x" (the default). I switched back to Firefox 3.x and the problem disappeared without any further tuning => something is not right for 4.0 beta.

Comment 3

[Boris Zbarsky [:bzbarsky]]

I'm not sure why this ever worked for you.  Temp files during download are created using 0600 for the permissions, because they often live in a world-readable directory, so making them world-readable would be a privacy hole.  This has been the case going back to Firefox 3.0.

When the download is done, the file is moved from the temp location to the final location.  This doesn't change the permissions, of course.

Comment 4

[Boris Zbarsky [:bzbarsky]]

That said, I do see the behavior change from 3.6 to 4.0.  Interesting....

Comment 5

[Boris Zbarsky [:bzbarsky]]

The behavior changed back in July in this range: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=1ac07fe5f6c9&tochange=0f5fc40c6a0f

That's when we stopped using a custom file object implementation for OSX and started just using the generic Unix one.

The old implementation was just buggy (in an insecure way, in this case): it ignored the permissions mask passed to the Create() call.

I'm pretty sure we have existing bugs about the fact that umask is not applied properly to files in this situation; the problem is that there is no sane way to apply the umask when moving a file.

Comment 6

[Matthias Versen [:Matti]]

bug 612209 looks like a dupe but I can't find the umask/move case

Comment 8

[Boris Zbarsky [:bzbarsky]]

> bug 612209 looks like a dupe

Yeah, of this bug.  We have much older bugs on Linux for the behavior, iirc.

Comment 9

[Matthias Versen [:Matti]]

do you mean bug 124307 ?

Comment 10

[Boris Zbarsky [:bzbarsky]]

Aha!  Yes, indeed.  Looks like we need to do that in the OS X code too.  Thanks!

Comment 11

[Boris Zbarsky [:bzbarsky]]

Attachment: Make sure to fix the file permissions on Mac when downloading, now that mac's nsILocalFile::Create actually pays attention to permissions.

Comment 12

[Boris Zbarsky [:bzbarsky]]

Sadly, I have no idea how to add a regression test for this.

Comment 13

[Boris Zbarsky [:bzbarsky]]

Pushed http://hg.mozilla.org/mozilla-central/rev/7cab48d6f673