Permissions should not be world-readable for profile directory

RESOLVED INCOMPLETE

Status

--
major
RESOLVED INCOMPLETE
18 years ago
3 years ago

People

(Reporter: ajp+mozilla, Unassigned)

Tracking

({fixed-aviary1.0, fixed1.4.2, privacy})

Trunk
x86
Linux
fixed-aviary1.0, fixed1.4.2, privacy
Bug Flags:
blocking1.4.2 +
blocking1.7.5 +
blocking-aviary1.0PR +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [have patch] need1.7.x)

Attachments

(5 attachments)

(Reporter)

Description

18 years ago
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."

Updated

18 years ago
Status: NEW → ASSIGNED
Target Milestone: --- → Future

Updated

18 years ago
Summary: Permissions should not be world-readable for password files → [z]Permissions should not be world-readable for password files

Updated

18 years ago
Summary: [z]Permissions should not be world-readable for password files → Permissions should not be world-readable for password files
Whiteboard: [z]

Comment 1

18 years ago
.
Assignee: morse → nobody
Status: ASSIGNED → NEW
Whiteboard: [z]

Comment 2

17 years ago
this is probably at least somewhat important to fix.
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.
Assignee: nobody → morse
Keywords: privacy
*** Bug 116745 has been marked as a duplicate of this bug. ***

Comment 5

17 years ago
> 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.)
Severity: normal → major
Summary: Permissions should not be world-readable for password files → Permissions should not be world-readable for sensitive profile data
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

17 years ago
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?).
Assignee: morse → asa
Component: Password Manager → Browser-General
QA Contact: tpreston → doronr
Summary: Permissions should not be world-readable for sensitive profile data → Permissions should not be world-readable for profile directory

Comment 8

17 years ago
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

17 years ago
.
Assignee: asa → ccarlen
Component: Browser-General → Profile Manager BackEnd
QA Contact: doronr → ktrina

Comment 10

17 years ago
OK, I'll take a whack at this soon.
Assignee: ccarlen → jmd
Target Milestone: Future → mozilla1.2alpha

Comment 11

16 years ago
*** Bug 179359 has been marked as a duplicate of this bug. ***
jmd?  Are you actually working on this?

Comment 13

16 years ago
.
Assignee: jmd → nobody
Over to security.  I still feel that this is a critical bug that needs to be 
fixed ASAP...
Assignee: nobody → mstoltz
Component: Profile Manager BackEnd → Security: General
Priority: P3 → --
QA Contact: ktrina → carosendahl
Target Milestone: mozilla1.2alpha → ---
I agree that this is a major problem. Over to Profile Manager; let's get this
fixed for 1.4.
Assignee: mstoltz → ccarlen
Component: Security: General → Profile Manager BackEnd
QA Contact: carosendahl → gbush

Comment 16

16 years ago
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.



Flags: blocking1.6a?
Flags: blocking1.4.2?
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...
Flags: blocking1.6b?
um
so is this bug about ~/.mozilla as the component & summary says, or about
*.s/*.w files, as comment 0 says?

Created attachment 134404 [details] [diff] [review]
address comment 0

changes the permissions of .s/.w to 0600

Updated

15 years ago
Attachment #134404 - Flags: review+
Created attachment 134405 [details] [diff] [review]
mostly address summary

for some reason, ~/.mozilla itself still has rx for group and others... the
actual profile directory does not, though
This bug should be for everything in .mozilla -- none of those files or
directories should be world-readable.  Security in depth, you know.
Created attachment 134412 [details] [diff] [review]
shaver's suggestion

thanks shaver, that worked
Comment on attachment 134412 [details] [diff] [review]
shaver's suggestion

r=ccarlen
Attachment #134412 - Flags: review?(ccarlen) → review+
Is this patch ready to land or do we need additional reviews?
Comment on attachment 134404 [details] [diff] [review]
address comment 0

we need super-review too, and review for one patch here
Attachment #134404 - Flags: superreview?(bz-vacation)
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?
Attachment #134412 - Flags: superreview?(bz-vacation)
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... :(
Attachment #134412 - Flags: superreview?(bz-vacation) → superreview+
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.
Attachment #134404 - Flags: superreview?(bz-vacation) → superreview+

Updated

15 years ago
Attachment #134405 - Flags: superreview?(bz-vacation)
Attachment #134405 - Flags: review?(ccarlen)
Attachment #134405 - Flags: review+
Comment on attachment 134405 [details] [diff] [review]
mostly address summary

sr=bzbarsky assuming this has been carefully tested...
Attachment #134405 - Flags: superreview?(bz-vacation) → superreview+
Comment on attachment 134412 [details] [diff] [review]
shaver's suggestion

a=asa (on behalf of drivers) for checkin to 1.6beta
Attachment #134412 - Flags: approval1.6b+
Comment on attachment 134405 [details] [diff] [review]
mostly address summary

Oops, wrong patch. a=asa (on behalf of drivers) for checkin to 1.6b
Attachment #134405 - Flags: approval1.6b+
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

Updated

15 years ago
Flags: blocking1.6b?
a) which of these patches actually needs to go in to 1.4.2

b) why wasn't this bug marked fixed?
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
a=mkaply for whatever patches need to go into 1.4.2.

Please put them in and mark fixed1.4.2.

Thanks
Flags: blocking1.4.2? → blocking1.4.2+
all three patches checked in on the 1.4 branch.
Keywords: fixed1.4.2

Comment 38

15 years ago
Created attachment 153688 [details] [diff] [review]
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.
Attachment #153688 - Flags: review?(dwitte)

Comment 39

15 years ago
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).
Attachment #153688 - Flags: superreview?(bzbarsky)
Attachment #153688 - Flags: review?(dwitte)
Attachment #153688 - Flags: review+

Comment 40

15 years ago
i've checked in a change to trunk to make prefs use 0600 perms when writing, as
part of bug 250092.
Comment on attachment 153688 [details] [diff] [review]
cookies.txt permissions need improvement too

sr=bzbarsky.  dwitte, can you land this?
Attachment #153688 - Flags: superreview?(bzbarsky) → superreview+

Comment 42

15 years ago
Created attachment 153745 [details] [diff] [review]
use 0600 for cookies.txt & hostperm.1

this is what i checked in to trunk (i did hostperm.1 too). thanks for the
patch.
Flags: blocking-aviary1.0RC1+
Flags: blocking1.7.2+

Comment 43

15 years ago
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
Assignee: ccarlen → dwitte

Updated

15 years ago
Whiteboard: [have patch]

Comment 44

15 years ago
checked in attachment 153745 [details] [diff] [review] to aviary1.0.
Keywords: fixed-aviary1.0
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.
Whiteboard: [have patch] → [have patch] need1.7.x
Can someone please just tell me what needs to go into 1.7 and I will do it?

Comment 48

12 years ago
-> default owner
Assignee: dwitte → nobody
QA Contact: agracebush → profile-manager-backend
What's the status here, can this be closed?
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.

Updated

6 years ago
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → INCOMPLETE
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.