Opened 15 years ago

Last modified 3 years ago

#600 defect closed fixed (fixed)

setgroups() raises when given too many groups, and nothing handles the error

Reported by: Jean-Paul Calderone Owned by:
Priority: highest Milestone:
Component: Keywords:
Cc: Glyph, radix, Jean-Paul Calderone, itamarst, Tv, Moshe Zadka, jknight Branch:
Author:

Description


Attachments (1)

setgroups.patch (2.5 KB) - added by Jean-Paul Calderone 15 years ago.

Download all attachments as: .zip

Change History (12)

comment:1 Changed 15 years ago by Jean-Paul Calderone

2004/04/17 14:11 MDT [-]   File
"/home/radix/Projects/Twisted/twisted/scripts/twistd.py", line 160, in
startApplication
2004/04/17 14:11 MDT [-]     shedPrivileges(config['euid'], process.uid,
process.gid)
2004/04/17 14:11 MDT [-]   File
"/home/radix/Projects/Twisted/twisted/scripts/twistd.py", line 134, in
shedPrivileges
2004/04/17 14:11 MDT [-]     switchUID(uid, gid, euid)
2004/04/17 14:11 MDT [-]   File
"/home/radix/Projects/Twisted/twisted/python/util.py", line 600, in switchUID
2004/04/17 14:11 MDT [-]     initgroups(uid, gid)
2004/04/17 14:11 MDT [-]   File
"/home/radix/Projects/Twisted/twisted/python/util.py", line 578, in initgroups
2004/04/17 14:11 MDT [-]     setgroups(l)
2004/04/17 14:11 MDT [-] ValueError: too many groups

comment:2 Changed 15 years ago by Glyph

Is this bug still outstanding?

comment:3 Changed 15 years ago by Tv

Yes and no. As much as it ever was :)

From the bug report:

"/home/radix/Projects/Twisted/twisted/python/util.py", line 578, in initgroups
2004/04/17 14:11 MDT [-]     setgroups(l)
2004/04/17 14:11 MDT [-] ValueError: too many groups

There is no code in twisted.python.util that would protect that
setgroups from raising ValueErrors to the caller.

Then again, this is a case of running against an OS-imposed limit,
and I think that code MUST fail. Whether it should catch ValueError
and raise something like TooManyGroupsError is another issue.

Changed 15 years ago by Jean-Paul Calderone

Attachment: setgroups.patch added

comment:4 Changed 15 years ago by Jean-Paul Calderone

Here's part of a "fix".

I think the rest of the fix comes in two parts:

  Don't even try to setgroups() if uid != 0

  If Application is passed None for uid and gid, don't even call initgroups().

comment:5 Changed 15 years ago by jknight

Target the uid/gid changes for 2.0.

comment:6 Changed 15 years ago by Jean-Paul Calderone

Resolved by 13136

comment:7 Changed 15 years ago by Tv

How is this fixed by that r13136? I don't see it doing anything else except not
calling switchUID by default, and thus, making the bug appear less often.
If you use --uid or --gid, and the users belongs to too many groups, it'll still
fail in exactly the same way.

comment:8 Changed 15 years ago by jknight

OMG, this is the code from glibc in initgroups():
  do
    result = setgroups (ngroups, groups);
  while (result == -1 && errno == EINVAL && --ngroups > 0);

That is fucking evil. I'll commit a fix to twisted to do the same.

comment:9 Changed 15 years ago by Tv

Recording this just for future reference: the commit that actually "fixed"
(read: covered up) things was r13140. Sorry for the noise, but I want that here
or I'll keep re-searching the right commit all the time.

comment:10 Changed 9 years ago by <automation>

Owner: Jean-Paul Calderone deleted

comment:11 Changed 3 years ago by Craig Rodrigues <rodrigc@…>

In 91ff8029:

Merge pull request #600 from twisted/8913-htmlizer-rodrigc

Author: rodrigc
Reviewer: glyph
Fixes: #8913

Port pyhtmlizer to Python 3

Note: See TracTickets for help on using tickets.