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 20:43:31 +1000
From: Graham Dumpleton <graham.dumpleton@...il.com>
To: Solar Designer <solar@...nwall.com>
Cc: oss-security@...ts.openwall.com
Subject: Re: Security release for mod_wsgi (version 3.5)

I saw the email as it popped up in my twitter feed of all places.

Am about to make a release which improves the error handling and the one off error.

Graham

On 18/06/2014, at 8:03 PM, Solar Designer <solar@...nwall.com> wrote:

> 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