235781 - XPInstall ignores user's umask when installing files

Status: RESOLVED FIXED

(Core Graveyard :: Installer: XPInstall Engine, defect)

Description

[Andrew Schultz]

When files are installed via XPInstall, the user's umask is ignored.  The
XPInstall engine should respect the user's umask setting.  The problem code is here:

nsZipArchive::ExtractFile
http://lxr.mozilla.org/mozilla/source/modules/libjar/nsZipArchive.cpp#641
nsJAR::Extract
http://lxr.mozilla.org/mozilla/source/modules/libjar/nsJAR.cpp#251

both open the file with 0644 perms and then chmod the file to the appropriate
perms, but this sidesteps any umask.

Just to make things more complicated, the nsZipArchive is part of standalone
libjar, where PR_Open is defined as fopen in zipstub.h, so passing the file's
mode to PR_Open wouldn't work in that situation.

this bug contributed to problems in bug 231083.

Comment 1

[Andrew Schultz]

also note that (as argued against in bug 111124), directories are created with
appropriately umasked permissions here:
http://lxr.mozilla.org/seamonkey/source/xpinstall/src/nsInstallFile.cpp#213

Comment 2

[Andrew Schultz]

Attachment: dbaron's patch minus a chmod

this is dbaron's patch from bug 231083 except that this patch also removes the
chmod from nsJAR::Extract.  This would fix things except for the standalone
libjar/PR_Open problem.

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I'd actually wanted to add an nsIFile::Remove / PR_Delete before the PR_Open /
GetANSIFileDesc calls so that it would be creating a new file rather than
reusing an old one that might have different permissions.  Otherwise a XPI
couldn't fix permissions that were wrong in an old buggy one, I think.

Comment 4

[Andrew Schultz]

Attachment: add PR_TRUNCATE and workaround for standalone

this adds PR_TRUNCATE to the flags so that permissions are set for existing
files.	

this patch also grabs the umask and uses it to chmod the extracted file when
using standalone libjar.

Comment 5

[Benjamin Smedberg]

Comment on attachment 142413 [details] [diff] [review]
add PR_TRUNCATE and workaround for standalone

I don't know enough about *nix file systems and umask sto review this.
Leaf/dbaron/dveditz?

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

How does this work around the problem for standalone?  Anyway, I attached a
patch that does this the cheap way to bug 231803.

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

er, bug 231083.

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Oh, I do see the workaround for standalone now, but I'd prefer that in a PR_Open.

Did you test that PR_TRUNCATE causes the permissions to be used if the file
exists?  Based on the documentation of open(2), I'd guess that it wouldn't do so.

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(In reply to comment #8)
> Oh, I do see the workaround for standalone now, but I'd prefer that in a PR_Open.

Actually, never mind, that's hard.

Comment 10

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

But I'll attach a slightly improved version of this patch to bug 231083 tomorrow.

Comment 11

[Andrew Schultz]

(In reply to comment #8)
> Did you test that PR_TRUNCATE causes the permissions to be used if the file
> exists?  Based on the documentation of open(2), I'd guess that it wouldn't do so.

The documentation itself is ambiguous, but it worked ok when I actually tested
it (I can't retest right now).  It might not be portable though.

Comment 12

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

It doesn't seem portable to Fedora Core 2.  I just did the following test.  I
added this to a test program:

PR_Open("/tmp/mozpermtest", PR_WRONLY | PR_CREATE_FILE | PR_TRUNCATE, 0700);

ran the test program, and it created /tmp/mozpermtest with 0700 (my umask is 002).

Then I changed the line to this:

PR_Open("/tmp/mozpermtest", PR_WRONLY | PR_CREATE_FILE | PR_TRUNCATE, 0666);

and ran the test program again, and the file was still 0700.

Comment 13

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Fixed by checkin of patch on bug 231083 to trunk, 2004-08-16 17:12 -0700.

Comment 14

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Fixed by checkin of bug 231083 to MOZILLA_1_7_BRANCH, 2004-08-17 13:24 -0700.

Comment 15

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

Comment on attachment 142413 [details] [diff] [review]
add PR_TRUNCATE and workaround for standalone

so this review request is obsolete, right?