365703 - setgid programs linked with NSPR allow elevation of privileges

Status: RESOLVED FIXED

(NSPR :: NSPR, defect)

Description

[Mike Hommey [:glandium]]

This affects all unices systems.

(ashamedly copy/paste from bug #351470)
NSPR logging is controlled with a couple of environment variables, 
one to enable it, and a second to control the name of the log file.
This appears to all work in "optimized" (non-debug) builds.  

So, if any setgid $group program is linked with NSPR, any user can clobber
any file on the system (any $group writable file) by setting NSPR's 
environment variables to log to that file, and then running a setgid $group
program linked with NSPR.

For example, on Debian, there is a program called camel-lock-helper-1.2 that comes with evolution-data-server, that is setgid mail. Typically, that means files in /var/mail can be clobbered by anyone can run camel-lock-helper-1.2...

Testing for getgid() == getegid() wherever getuid() == geteuid() is tested in the fix for bug #351470 should be enough to fix this.

Comment 1

[Wan-Teh Chang]

Attachment: Proposed patch

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 250501 [details] [diff] [review]
Proposed patch

additional review, r=nelson

Comment 3

[Mike Hommey [:glandium]]

Comment on attachment 250501 [details] [diff] [review]
Proposed patch

>-        if (getuid() != geteuid()) {
>+        if (getuid() != geteuid() || getgid() != getegid()) {

Wouldn't it be better to add a level of parenthesis ?

Comment 4

[Wan-Teh Chang]

Comment on attachment 250501 [details] [diff] [review]
Proposed patch

Mike, I'll be happy to add a level of parenthesis when I check in the patch.

Do you have the setup to trigger this bug
and test this patch?

Comment 5

[Mike Hommey [:glandium]]

I'll give it a try this week-end. I have to fix this in libnspr for Debian(we build it out of xulrunner, for the moment)

Comment 6

[Wan-Teh Chang]

Attachment: Proposed patch (as checked in)

I added a level of parenthesis as Mike suggested.  I checked
in the patch on the NSPR trunk (NSPR 4.7)

Checking in io/prlog.c;
/cvsroot/mozilla/nsprpub/pr/src/io/prlog.c,v  <--  prlog.c
new revision: 3.37; previous revision: 3.36
done
Checking in misc/prtrace.c;
/cvsroot/mozilla/nsprpub/pr/src/misc/prtrace.c,v  <--  prtrace.c
new revision: 3.11; previous revision: 3.10
done

Checking in io/prlog.c;
/cvsroot/mozilla/nsprpub/pr/src/io/prlog.c,v  <--  prlog.c
new revision: 3.24.2.10; previous revision: 3.24.2.9
done
Checking in misc/prtrace.c;
/cvsroot/mozilla/nsprpub/pr/src/misc/prtrace.c,v  <--  prtrace.c
new revision: 3.5.4.5; previous revision: 3.5.4.4
done

Checking in io/prlog.c;
/cvsroot/mozilla/nsprpub/pr/src/io/prlog.c,v  <--  prlog.c
new revision: 3.34.2.2; previous revision: 3.34.2.1
done
Checking in misc/prtrace.c;
/cvsroot/mozilla/nsprpub/pr/src/misc/prtrace.c,v  <--  prtrace.c
new revision: 3.8.2.2; previous revision: 3.8.2.1
done

Comment 7

[Wan-Teh Chang]

Comment on attachment 250638 [details] [diff] [review]
Proposed patch (as checked in)

I forgot to say: the patch was also checked in on the NSPRPUB_PRE_4_2_CLIENT_BRANCH (Mozilla 1.9 Alpha 2) and NSPR_4_6_BRANCH (NSPR 4.6.5).

Comment 8

[Mike Hommey [:glandium]]

Unfortunately, the incriminated setgid program is actually linked against libnspr for no reason: it doesn't use it. I tried some other programs but can't manage to make them generate a log file... :-/

Comment 9

[Alexei Volkov]

Comment on attachment 250638 [details] [diff] [review]
Proposed patch (as checked in)

r=alexei.volkov.bugs

Comment 10

[Myk Melez [:myk] [@mykmelez]]

Does this also affect the 1.8 branch?

Comment 11

[Wan-Teh Chang]

Myk: the Mozilla 1.8 branch already has the fix.  You can verify
that by checking the value of NSPR_CO_TAG in mozilla/client.mk.
It is NSPR_4_6_5_RTM, which is the NSPR release the fix first
appeared in (the target milestone of this bug is "4.6.5").  The
Mozilla 1.8.0 branchy also has the fix.

Comment 12

[Myk Melez [:myk] [@mykmelez]]

Wan-Teh: great, thanks for the info!  Marking fixed1.8 so this no longer shows up on triage queries for bugs that might need to be checked into the branch.