Re: Security release for mod_wsgi (version 3.5)

[Graham Dumpleton <graham.dumpleton () gmail com>]

On 17/06/2014, at 9:27 PM, Tomas Hoger <thoger () redhat com> wrote:

> On Wed, 21 May 2014 11:46:32 +0200 Kurt Seifried wrote:
>
> > So CVEs were assigned, this is now public, very well written an
> > detailed write up is at:
> >
> > http://blog.dscpl.com.au/2014/05/security-release-for-modwsgi-version-35.html
>
> ...
>
> > Issue: Possibility of local privilege escalation when using daemon
> > mode. (CVE-2014-0240)
> >
> > The issue is believed to affect Linux systems running kernel versions
> > > = 2.6.0 and < 3.1.0.
> >
> > The issue affects all versions of mod_wsgi up to and including version
> > 3.4.
> >
> > The source of the issue derives from mod_wsgi not correctly handling
> > Linux specific error codes from setuid(), which differ to what would
> > be expected to be returned by UNIX systems conforming to the Open
> > Group UNIX specification for setuid().
>
> Looking at the patch, mod_wsgi was previously expecting that setuid may
> return error, it only failed to respond to the failure correctly.  It
> only logged information about the failure, and continued to run with
> unexpected privileges.
>
> Few lines above the patched code, the same pattern is used for setgid
> and setgroups / initgroups calls.  Is there a reason to not patch those
> in the same way?  While there may be no such easy way to trigger failure
> for those, their failure would also lead to user code running with
> unexpected privileges.

Looking first at the setgid() case, guess it depends on what circumstances EINVAL would actually be returned, something 
which the manual pages for setgid() don't really clearly lay out.

On MacOS X at least, it really doesn't care what you give it. You can give it negative numbers or even really large 
numbers and it will happily set it to the value. I can't find any good information on what Linux would do and can't 
test it right now.

As it is, the gid value used as input to that is derived from the 'group' option to the mod_wsgi WSGIDaemonProcess 
directive.

This option can be a group name in which case it has to survive the call getgrnam() to determine the actual gid for the 
group.

If that call fails, Apache rather nastily exits the process, which would occur when the configuration is being 
processed in the Apache parent process.

AP_DECLARE(gid_t) ap_gname2id(const char *name)
{
    struct group *ent;

    if (name[0] == '#')
        return (atoi(&name[1]));

    if (!(ent = getgrnam(name))) {
        ap_log_error(APLOG_MARK, APLOG_STARTUP, 0, NULL, APLOGNO(00544)
                     "%s: bad group name %s", ap_server_argv0, name);
        exit(1);
    }

    return (ent->gr_gid);
}

If that succeeds and returns a gid, then presumably it is valid and should always subsequently work, unless the kernel 
can change its mind at the point of the setgid() call based on whether it can find a group name that maps to it, which 
I don't believe it would check. So one would have to take it that the gid at that point should work.

So via that path whereby a group name is provided the possibility of that yielding EINVAL at the call for setgid() is 
probably not going to occur.

The path to worry about more is the ability to use #nnn and set an arbitrary integer number.

At this point it is bordering on if you provide it garbage values, then expect the unexpected.

So I agree that it is probably better to exit, but in practice unless someone is going out to deliberately feed it 
really strange values for the group as #nnn, they should not encounter it.

I will improve the handling of that one, but I don't see it as significant enough to warrant making any big noise about 
it.

As for initgroups() it can supposedly only fail if you aren't root. That code will only ever be executed as root, so 
shouldn't expect to see it fail. The logging of a message was more just in case something strange happened.

Finally for setgroups(). This is for supplementary group lists and is used by 'supplementary-groups' option to 
WSGIDaemonProcess directive.

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)
                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.

The later error message was again more just in case something strange happened.

So just to be safe one could in all cases exit anyway, but I believe the possibility that these could cause an issue is 
extremely limited, with the only case being where the user provides a bad gid for #nnn to the 'group' option which did 
actually exceed some integer range which was actually enforced by the operating system being used. If they used an 
actual group name, which would be the typical case, they shouldn't be able to trigger a problem.

Graham