Openwall GNU/*/Linux - a small security-enhanced Linux distro for servers
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Wed, 18 Jun 2014 14:03:59 +0400
From: Solar Designer <solar@...nwall.com>
To: oss-security@...ts.openwall.com
Cc: Graham Dumpleton <graham.dumpleton@...il.com>
Subject: Re: Security release for mod_wsgi (version 3.5)

On Wed, Jun 18, 2014 at 08:08:10PM +1200, Matthew Daley wrote:
> I may be wrong as I haven't been following this discussion entirely, but...

I think Graham is not on oss-security.  CC added.

Graham, please comment on the potential off-by-one bug reported by
Matthew below:

> On Wed, Jun 18, 2014 at 12:39 AM, Graham Dumpleton
> <graham.dumpleton@...il.com> wrote:
> > This feature was added for one specific user and wouldn't be a well known feature unless people were reading change notes diligently as don't believe it is even covered in the documentation.
> >
> > Given that this code also only executes as root, the only error which could technically arise in this code for setgroups() is if the number of groups exceeded NGROUPS_MAX.
> >
> > This should not occur though as the number of groups was previously validated when the configuration was read:
> >
> >     if (groups_list) {
> >         const char *group_name = NULL;
> >         long groups_maximum = NGROUPS_MAX;
> >         const char *items = NULL;
> >
> > #ifdef _SC_NGROUPS_MAX
> >         groups_maximum = sysconf(_SC_NGROUPS_MAX);
> >         if (groups_maximum < 0)
> >             groups_maximum = NGROUPS_MAX;
> > #endif
> >         groups = (gid_t *)apr_pcalloc(cmd->pool,
> >                                       groups_maximum*sizeof(groups[0]));
> >
> >         groups[groups_count++] = gid;
> >
> >         items = groups_list;
> >         group_name = ap_getword(cmd->pool, &items, ',');
> >
> >         while (group_name && *group_name) {
> >             if (groups_count > groups_maximum)
> 
> This is an off-by-one error, isn't it? As in, it should be testing for
> groups_count >= groups_maximum and not the current test.
> 
> >                 return "Too many supplementary groups WSGI daemon process";
> >
> >             groups[groups_count++] = ap_gname2id(group_name);
> >             group_name = ap_getword(cmd->pool, &items, ',');
> >         }
> >     }
> >
> > Thus was pre-validated input.
> 
> - Matthew Daley

Powered by blists - more mailing lists

Your e-mail address:

Please check out the Open Source Software Security Wiki, which is counterpart to this mailing list.

Powered by Openwall GNU/*/Linux - Powered by OpenVZ