Closed Bug 59557 Opened 22 years ago Closed 10 years ago

Permissions should not be world-readable for profile directory

Categories

(Core Graveyard :: Profile: BackEnd, defect)

x86
Linux
defect
Not set
major

Tracking

(Not tracked)

RESOLVED INCOMPLETE

People

(Reporter: ajp+mozilla, Unassigned)

References

Details

(Keywords: fixed-aviary1.0, fixed1.4.2, privacy, Whiteboard: [have patch] need1.7.x)

Attachments

(5 files)

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."
Status: NEW → ASSIGNED
Target Milestone: --- → Future
Summary: Permissions should not be world-readable for password files → [z]Permissions should not be world-readable for password files
Summary: [z]Permissions should not be world-readable for password files → Permissions should not be world-readable for password files
Whiteboard: [z]
.
Assignee: morse → nobody
Status: ASSIGNED → NEW
Whiteboard: [z]
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. ***
> 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.
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
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.
.
Assignee: asa → ccarlen
Component: Browser-General → Profile Manager BackEnd
QA Contact: doronr → ktrina
OK, I'll take a whack at this soon.
Assignee: ccarlen → jmd
Target Milestone: Future → mozilla1.2alpha
*** Bug 179359 has been marked as a duplicate of this bug. ***
jmd?  Are you actually working on this?
.
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
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?
Blocks: 224065
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?

changes the permissions of .s/.w to 0600
Attachment #134404 - Flags: review+
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.
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+
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
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
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 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+
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+
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+
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
Whiteboard: [have patch]
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?
-> 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.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → INCOMPLETE
Product: Core → Core Graveyard

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

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

You need to log in before you can comment on or make changes to this bug.