Christian Perrier
2007-Jan-17 07:39 UTC
Re: [Pkg-shadow-devel] Bug#407231: passwd: users may gain system group access on package installation by coincidence
reassign 407231 adduser retitle 407231 adduser: with addgroup, users may gain system group access on package installation by coincidence thanks Quoting Leonard NorrgÄrd (vinsci@refactor.fi):> Package: passwd > Version: 1:4.0.18.1-6 > Severity: critical > Tags: security > Justification: root security hole > > > An ordinary user may end up with group ownership of system files > in the following scenario [2]: > > 1. A user is added, and receives the user and group ids, <name>. > 2. Later, a package is installed that asks for an identically named > system group to be created, using ''addgroup --system <name>''. > 3. Addgroup returns with a success exit status, showing the message > ''The group `<name>'' already exists as a system group. Exiting.", > even though the pre-existing <name> group, as a group added for > a user has a non-system id (ie. outside the range 100-999 [1]. > 4. The user <name> now has access to all system files that are > installed for the <name> group. > > The problem occurs because in /usr/sbin/addgroup, the code on/after > line 247 to existing_group_ok fails to check for and handle > the situation where the existing GID is outside of the system GID > boundaries. > > [1] http://www.debian.org/doc/debian-policy/ch-opersys.html#s9.2.2) > [2] I discovered this while working on the packaging for kvm, which > will create a ''kvm'' group, likely to collide with existing user > id:s on some systems.Thanks for your detailed explanations and bug report. I won''t go into the details, essentially because this bug report is misdirected. At first glance, you seem to be right and the bug seems easy to handle. You identified the bug as a bug in the "addgroup" utility. However "dpkg -S /usr/sbin/addgroup" will show you that this utility belongs to the "adduser" package, not passwd. I''m therefore reassigning this bug to adduser. Again, thanks a lot for your care investigating this issue. _______________________________________________ Adduser-devel mailing list Adduser-devel@lists.alioth.debian.org http://lists.alioth.debian.org/mailman/listinfo/adduser-devel
Debian Bug Tracking System
2007-Jan-17 07:59 UTC
[Adduser-devel] Processed: Re: [Pkg-shadow-devel] Bug#407231: passwd: users may gain system group access on package installation by coincidence
Processing commands for control@bugs.debian.org:> reassign 407231 adduserBug#407231: passwd: users may gain system group access on package installation by coincidence Bug reassigned from package `passwd'' to `adduser''.> retitle 407231 adduser: with addgroup, users may gain system group access on package installation by coincidenceBug#407231: passwd: users may gain system group access on package installation by coincidence Changed Bug title.> thanksStopping processing here. Please contact me if you need assistance. Debian bug tracking system administrator (administrator, Debian Bugs database)
Debian Bug Tracking System
2007-Jan-17 13:04 UTC
Processed: Re: [Adduser-devel] Processed: Re: [Pkg-shadow-devel] Bug#407231: passwd: users may gain system group access on package installation by coincidence
Processing commands for control@bugs.debian.org:> tags #407231 confirmedBug#407231: adduser: with addgroup, users may gain system group access on package installation by coincidence Tags were: security Tags added: confirmed> thanksStopping processing here. Please contact me if you need assistance. Debian bug tracking system administrator (administrator, Debian Bugs database)
Marc Haber
2007-Jan-17 13:24 UTC
[Adduser-devel] Processed: Re: [Pkg-shadow-devel] Bug#407231: passwd: users may gain system group access on package installation by coincidence
tags #407231 confirmed thanks I can reproduce this bug on sid adduser and have written (and committed) a test suite case to catch this. I _think_ that this patch fixes the issue: Index: adduser ==================================================================--- adduser (revision 689) +++ adduser (working copy) @@ -244,11 +244,11 @@ ################# if ($action eq "addsysgroup") { # Check if requested group already exists and we can exit safely - if (existing_group_ok($new_name, $new_gid) == 1) { + if (existing_group_ok($new_name, $new_gid) == 2) { printf (gtx("The group `%s'' already exists as a system group. Exiting.\n"), $new_name) if $verbose; exit 0; } - if (existing_group_ok($new_name, $new_gid) == 2) { + if (existing_group_ok($new_name, $new_gid) == 1) { printf (gtx("The group `%s'' already exists, but has a different GID. Exiting.\n"), $new_name) if $verbose; exit 1; } @@ -695,21 +695,21 @@ # returns 0 if the group doesn''t exist or # returns 1 if the group already exists with the specified gid (or $new_gid wasn''t specified) -# returns 2 if the group already exists, but $new_gid doesn''t match its gid +# returns 2 if the group already exists as a system group sub existing_group_ok { my($new_name,$new_gid) = @_; my ($dummy1,$dummy2,$gid); if (($dummy1,$dummy2,$gid) = getgrnam($new_name)) { + if( $gid >= $config{"first_system_gid"} && + $gid <= $config{"last_system_gid" } ) { + return 2; + } if( defined($new_gid) && $gid == $new_gid ) { return 1; } if (! defined($new_gid)) { return 1; } - if( $gid >= $config{"first_system_gid"} && - $gid <= $config{"last_system_gid" } ) { - return 2; - } } else { return 0; } I would like the people who are more knowledgeable with that part of the code to comment before I commit this. Greetings Marc -- ----------------------------------------------------------------------------- Marc Haber | "I don''t trust Computers. They | Mailadresse im Header Mannheim, Germany | lose things." Winona Ryder | Fon: *49 621 72739834 Nordisch by Nature | How to make an American Quilt | Fax: *49 621 72739835
Stephen Gran
2007-Jan-17 13:32 UTC
Bug#407231: [Adduser-devel] Processed: Re: [Pkg-shadow-devel] Bug#407231: passwd: users may gain system group access on package installation by coincidence
This one time, at band camp, Marc Haber said:> I can reproduce this bug on sid adduser and have written (and > committed) a test suite case to catch this. > > I would like the people who are more knowledgeable with that part of > the code to comment before I commit this.The real problem (I think - reflected by the patch I just checked in) is that existing_group_ok is called with an uninitialized $new_gid. This makes existing_group_ok return 1 (that behavior is probably wrong in existing_group_ok, but can be fixed later - for now, we just need to make sure we initialize $new_group before calling it). I have checked in a fix that takes care to initialize $new_group before calling the function, and it looks like it works, although I haven''t yet had time to run the test suite on it. -- ----------------------------------------------------------------- | ,''''`. Stephen Gran | | : :'' : sgran@debian.org | | `. `'' Debian user, admin, and developer | | `- http://www.debian.org | ----------------------------------------------------------------- -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 189 bytes Desc: Digital signature Url : http://lists.alioth.debian.org/pipermail/adduser-devel/attachments/20070117/99062f08/attachment-0003.pgp
Marc Haber
2007-Jan-17 14:50 UTC
Bug#407231: [Adduser-devel] Processed: Re: [Pkg-shadow-devel] Bug#407231: passwd: users may gain system group access on package installation by coincidence
On Wed, Jan 17, 2007 at 12:12:52PM +0000, Stephen Gran wrote:> This one time, at band camp, Marc Haber said: > > I can reproduce this bug on sid adduser and have written (and > > committed) a test suite case to catch this. > > > > I would like the people who are more knowledgeable with that part of > > the code to comment before I commit this. > > The real problem (I think - reflected by the patch I just checked in) is > that existing_group_ok is called with an uninitialized $new_gid. This > makes existing_group_ok return 1 (that behavior is probably wrong in > existing_group_ok, but can be fixed later - for now, we just need to > make sure we initialize $new_group before calling it). I have checked > in a fix that takes care to initialize $new_group before calling the > function, and it looks like it works, although I haven''t yet had time to > run the test suite on it.The new test still fails with your patch applied, and my patch (diff attached) fixes it. I have not yet committed it since it is a pretty severe change to a function that is used in different places. I also think that existing_group_ok was kind of misdocumented. Greetings Marc --- adduser (revision 695) +++ adduser (working copy) @@ -255,11 +255,11 @@ } # Check if requested group already exists and we can exit safely - if (existing_group_ok($new_name, $new_gid) == 1) { + if (existing_group_ok($new_name, $new_gid) == 2) { printf (gtx("The group `%s'' already exists as a system group. Exiting.\n"), $new_name) if $verbose; exit 0; } - if (existing_group_ok($new_name, $new_gid) == 2) { + if (existing_group_ok($new_name, $new_gid) == 1) { printf (gtx("The group `%s'' already exists, but has a different GID. Exiting.\n"), $new_name) if $verbose; exit 1; } @@ -697,21 +697,21 @@ # returns 0 if the group doesn''t exist or # returns 1 if the group already exists with the specified gid (or $new_gid wasn''t specified) -# returns 2 if the group already exists, but $new_gid doesn''t match its gid +# returns 2 if the group already exists as a system group sub existing_group_ok { my($new_name,$new_gid) = @_; my ($dummy1,$dummy2,$gid); if (($dummy1,$dummy2,$gid) = getgrnam($new_name)) { + if( $gid >= $config{"first_system_gid"} && + $gid <= $config{"last_system_gid" } ) { + return 2; + } if( defined($new_gid) && $gid == $new_gid ) { return 1; } if (! defined($new_gid)) { return 1; } - if( $gid >= $config{"first_system_gid"} && - $gid <= $config{"last_system_gid" } ) { - return 2; - } } else { return 0; } -- ----------------------------------------------------------------------------- Marc Haber | "I don''t trust Computers. They | Mailadresse im Header Mannheim, Germany | lose things." Winona Ryder | Fon: *49 621 72739834 Nordisch by Nature | How to make an American Quilt | Fax: *49 621 72739835
Marc Haber
2007-Jan-17 15:37 UTC
Bug#407231: [Adduser-devel] Processed: Re: [Pkg-shadow-devel] Bug#407231: passwd: users may gain system group access on package installation by coincidence
On Wed, Jan 17, 2007 at 02:43:49PM +0100, Marc Haber wrote:> The new test still fails with your patch applied, and my patch (diff > attached) fixes it.Stupid me. With your new code, the test is successful. I have tweaked the test suite somewhat. Can somebody please go over the diff beteween 3.101 and current svn HEAD before I upload? Greetings Marc -- ----------------------------------------------------------------------------- Marc Haber | "I don''t trust Computers. They | Mailadresse im Header Mannheim, Germany | lose things." Winona Ryder | Fon: *49 621 72739834 Nordisch by Nature | How to make an American Quilt | Fax: *49 621 72739835
[Taking this from the BTS to the developer mailing list] This is adduser from current svn HEAD (as is, Stephen''s fix for #407231, not mine). $ grep addusertest117 /etc/group addusertest117:x:157: $ sudo addgroup --system addusertest117 The group `addusertest117'' already exists, but has a different GID. Exiting. $ echo $? 1 $ I think this is wrong. addusertest117 is already existing a system group, and the user has requested it to be created as a system group with no preference regarding UID. Thus, I''d expect addgroup to say "addusertest117 already exists as a system group" and exit with a zero exit code since the user''s request could be catered for (in fact, it was already fulfilled before). It we upload as is, we''ll break every package with a postinst that just does "addgroup --system foo". Or am I missing something here? Greetings Marc -- ----------------------------------------------------------------------------- Marc Haber | "I don''t trust Computers. They | Mailadresse im Header Mannheim, Germany | lose things." Winona Ryder | Fon: *49 621 72739834 Nordisch by Nature | How to make an American Quilt | Fax: *49 621 72739835