59557 - Permissions should not be world-readable for profile directory

Status: RESOLVED INCOMPLETE

(Core Graveyard :: Profile: BackEnd, defect)

Description

[Ari Pollak]

For ~/.mozilla/<profile>/*.s & *.w, the permissions are 644, but they should
probably be 600. Even though ~/.mozilla is set 700, it's good practice, "just in
case."

Comment 1

[Stephen P. Morse]

.

Comment 2

[Stuart Parmenter]

this is probably at least somewhat important to fix.

Comment 3

[Boris Zbarsky [:bzbarsky]]

using the nsOutputFileStream::nsOutputFileStream(nsIFileSpec* inSpec) 
constructor gives a file that has permissions (0666 & ~umask).  This is no good. 
 You really want to use NS_NewIOFileStream or something along those lines, and 
make all your methods take nsIOutputStream instead of the implementation class 
as an argument.

Reassigning back to morse.

Comment 4

[Mitchell Stoltz (not reading bugmail)]

*** Bug 116745 has been marked as a duplicate of this bug. ***

Comment 5

[Jeremy M. Dolan]

> Even though ~/.mozilla is set 700, it's good practice, "just in case."

WRONG. ~/.mozilla IS WORLD READABLE IN NEW PROFILES!

Given that that is no longer true, and pavlovs comment, increasing severity.

As this is a fairly severe security problem, and is futured, I can attempt a
patch perhaps soon. Can someone point me to where about these files are created?

*crosses fingers hoping not to hear "all over"*

(Also, if my summary rewording is not appropriate, my bug should not have been
duped.)

Comment 6

[Boris Zbarsky [:bzbarsky]]

Jeremy, the files are created when the write stream is opened.  This happens, as 
I mentioned, when the nsOutputFileStream is created.  This _does_ happen in a 
few places, though.

Comment 7

[Stephen P. Morse]

Who is to define what you mean by "sensitive" profile data?  What's 
insignificant to one user might be considered sensitive to another.  For 
example, the prefs.js file since it does contain the last page that you visited.

For that reason, changin summary from

   Permissions should not be world-readable for sensitive profile data

to

   Permissions should not be world-readable for profile directory

And now it's clear that this is not a "password manager" bug, so changing 
component to browser general (perhaps it should be profile manager?).

Comment 8

[Jeremy M. Dolan]

Good, that's easier for me to attempt a fix, anyway :)

I figured for whatever reason we wanted 4.x parity on the permissions, assuming
there was a reason those too were world readable on the directory. I can see
some wanting bookmarks/prefs to be available. The general rule of politeness on
UNIX is everything that can be should be world readable, but I'm not the biggest
fan of that.

Even with the directory 0700 though, it would be proper to still 0600 sensitive
files, for when a user manually opens the directory up. But that really is a
'Future' issue.

Comment 9

[timeless]

.

Comment 10

[Jeremy M. Dolan]

OK, I'll take a whack at this soon.

Comment 11

[Arthur]

*** Bug 179359 has been marked as a duplicate of this bug. ***

Comment 12

[Boris Zbarsky [:bzbarsky]]

jmd?  Are you actually working on this?

Comment 13

[Jeremy M. Dolan]

.

Comment 14

[Boris Zbarsky [:bzbarsky]]

Over to security.  I still feel that this is a critical bug that needs to be 
fixed ASAP...

Comment 15

[Mitchell Stoltz (not reading bugmail)]

I agree that this is a major problem. Over to Profile Manager; let's get this
fixed for 1.4.

Comment 16

[Asaf Amit]

The directory ~/.mozilla has the following permissions :
   rwxr-xr-x

This means that anyone can look into the content of the user profile.
This exposes all passwords stored in the NNNN.s file as well as 
email and other information.

requesting for blocking as basically all the user information is exposed.

Note that changing permission of ~/.mozilla should suffice to reduce the
priority of the issue.

Comment 17

[Boris Zbarsky [:bzbarsky]]

Could we get some traction on this?  All of our cleverness with salting and the
like is pretty pointless if we leave all this stuff with world-readable
permissions set...

Comment 18

[Christian :Biesinger (don't email me, ping me on IRC)]

um
so is this bug about ~/.mozilla as the component & summary says, or about
*.s/*.w files, as comment 0 says?

Comment 19

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: address comment 0

changes the permissions of .s/.w to 0600

Comment 20

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: mostly address summary

for some reason, ~/.mozilla itself still has rx for group and others... the
actual profile directory does not, though

Comment 21

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

http://lxr.mozilla.org/seamonkey/source/xpcom/io/nsAppFileLocationProvider.cpp#388
is why.  Make that 0700 and you're set, I think.

Comment 22

[Boris Zbarsky [:bzbarsky]]

This bug should be for everything in .mozilla -- none of those files or
directories should be world-readable.  Security in depth, you know.

Comment 23

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: shaver's suggestion

thanks shaver, that worked

Comment 24

[Conrad Carlen (not reading bugmail)]

Comment on attachment 134412 [details] [diff] [review]
shaver's suggestion

r=ccarlen

Comment 25

[Asa Dotzler [:asa]]

Is this patch ready to land or do we need additional reviews?

Comment 26

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 134404 [details] [diff] [review]
address comment 0

we need super-review too, and review for one patch here

Comment 27

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 134412 [details] [diff] [review]
shaver's suggestion

bz, if you super-review before the freeze, could you check the patches in too?

Comment 28

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 134412 [details] [diff] [review]
shaver's suggestion

sr=bzbarsky and checked in.

I'm leaving this open, since I stand behind comment 22.

I almost wish we could sanely change the permissions on all the hundreds of
thousands of machines where we already have insecure user profiles all nicely
set up...  Perhaps we should hack together some code that checks for a pref and
it it's not set changes the profile permissions and sets it?  It'll mess with
the few dozen people who have manually set their .mozilla permissions, but only
once... :(

Comment 29

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 134404 [details] [diff] [review]
address comment 0

I think we should do this too.	I've taken the liberty to check this patch in.

Comment 30

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 134405 [details] [diff] [review]
mostly address summary

sr=bzbarsky assuming this has been carefully tested...

Comment 31

[Asa Dotzler [:asa]]

Comment on attachment 134412 [details] [diff] [review]
shaver's suggestion

a=asa (on behalf of drivers) for checkin to 1.6beta

Comment 32

[Asa Dotzler [:asa]]

Comment on attachment 134405 [details] [diff] [review]
mostly address summary

Oops, wrong patch. a=asa (on behalf of drivers) for checkin to 1.6b

Comment 33

[Christian :Biesinger (don't email me, ping me on IRC)]

ok... attachment 134405 [details] [diff] [review] checked in:
Checking in nsProfile.cpp;
/cvsroot/mozilla/profile/src/nsProfile.cpp,v  <--  nsProfile.cpp
new revision: 1.294; previous revision: 1.293
done
Checking in nsProfileAccess.cpp;
/cvsroot/mozilla/profile/src/nsProfileAccess.cpp,v  <--  nsProfileAccess.cpp
new revision: 1.85; previous revision: 1.84
done

Comment 34

[Mike Kaply [:mkaply]]

a) which of these patches actually needs to go in to 1.4.2

b) why wasn't this bug marked fixed?

Comment 35

[Christian :Biesinger (don't email me, ping me on IRC)]

mkaply:
a) all of those are good for 1.4.2
b) it only addresses some of the files in the profile; some are still created
world-readable, which is why the bug is still open

Comment 36

[Mike Kaply [:mkaply]]

a=mkaply for whatever patches need to go into 1.4.2.

Please put them in and mark fixed1.4.2.

Thanks

Comment 37

[Christian :Biesinger (don't email me, ping me on IRC)]

all three patches checked in on the 1.4 branch.

Comment 38

[Pavel Kankovsky]

Attachment: cookies.txt permissions need improvement too

cookies.txt is created with default permissions, often group & world readable.
This is very bad, even with restrictive directory permissions. (patch for 1.7)

Moreover, it would be nice (albeit somewhat controversial) if Mozilla was able
to fix insecure permissions on existing profiles after-the-fact.

Comment 39

[[e-mail]]

Comment on attachment 153688 [details] [diff] [review]
cookies.txt permissions need improvement too

we'll do this for trunk, too (ignore the rot, this is simple enough).

Comment 40

[[e-mail]]

i've checked in a change to trunk to make prefs use 0600 perms when writing, as
part of bug 250092.

Comment 41

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 153688 [details] [diff] [review]
cookies.txt permissions need improvement too

sr=bzbarsky.  dwitte, can you land this?

Comment 42

[[e-mail]]

Attachment: use 0600 for cookies.txt & hostperm.1

this is what i checked in to trunk (i did hostperm.1 too). thanks for the
patch.

Comment 43

[chris hofmann]

needs to get on the aviary 1.0 branch as well.  can someone do that today?
mark with  fixed-aviary1.0 keyword when landed  --thanks

Comment 44

[[e-mail]]

checked in attachment 153745 [details] [diff] [review] to aviary1.0.

Comment 45

[Daniel Veditz [:dveditz]]

Also need checkin on the 1.7.x branch, or add "fixed1.7.x" keyword if already done.

Although all those "fixed<foo>" kewords are confusing if the main bug is still
open because it's incompletely fixed. We should start spinning off sub bugs so
we know what we've done and what we haven't.

Comment 46

[Mike Kaply [:mkaply]]

Can someone please just tell me what needs to go into 1.7 and I will do it?

Comment 47

[Christian :Biesinger (don't email me, ping me on IRC)]

mkaply: only attachment 153745 [details] [diff] [review] needs to land on 1.7.

Comment 48

[[e-mail]]

-> default owner

Comment 49

[Nochum Sossonko [:Natch]]

What's the status here, can this be closed?

Comment 50

[Daniel Veditz [:dveditz]]

As initially written (*.s and *.w files) this is half fixed -- the current descendant of *.s (signons3.txt) is 0600, the *.w equivalent formhistory.sqlite is 0644.

Profile directories, at least if Firefox creates them, are 0600. If you use the OS pickers to create a custom folder the folder will be 0644 (linux and mac, didn't test windows).

Most of the code is completely different, it might be best to start over in a new bug.

Comment 51

[flarn2006]

I'm using Firefox 90.0 and it still has this problem. Can this be reopened or should I submit it anew?

Comment 52

[flarn2006]

Actually, ~/.mozilla is 0755, which wasn't reported in the original bug. So this is more serious than the original report.