suexec enhancement

[Magdi Ahmed <ma...@garo.ca>]

Hi,

I host many sites on my server, and have found suEXEC to be a great benefit
for both security and keeping everything tidy.  The one problem I have
always had with it is that it cannot work with a shared script. So I
couldn't provide my virtual hosts with a common page counter script.

This caused me quite a headache over the years, it was either I used suexec
for a virtual host and copied the shared scripts to that hosts directory, or
didn't use suexec at all and run the risk of users executing insecure
scripts.

I finally resolved to modify suexec to include a shared directory root (that
still must be in the document root) along with a valid shared user/group.
suexec now bypasses some of the checks for scripts in that directory and
allows scripts to run from that directory if they are owned by the
designated user/group. I have included my modified suexec for your review
and comments.



Thank You,
Magdi Ahmed

Re: suexec enhancement

[Glenn <gs...@gluelogic.com>]

On Sat, Feb 07, 2004 at 08:35:35PM -0400, Magdi Ahmed wrote:
> I finally resolved to modify suexec to include a shared directory root (that
> still must be in the document root) along with a valid shared user/group.
> suexec now bypasses some of the checks for scripts in that directory and
> allows scripts to run from that directory if they are owned by the
> designated user/group. I have included my modified suexec for your review
> and comments.

A cursory look and I found at least two problems:

Even if sharedir is set, you should still make sure that the shared
directory is not _world_ writable.  So you might skip the check for
_group_ writable, but should still abort if the dir is world writable.
Same thing for aborting if target program is world writable.


Typical libc implementations reuse a static struct passwd (behind the
scenes) between calls to getpwnam/getpwuid.  This means that you need
to copy out information you plan to keep, or need to use the reentrant
getpwnam_r/getpwuid_r if available on your system.

Note below that you access pw->pw_uid, pw->pw_name, and pw->pw_dir
AFTER you have called getpwnam/getpwuid on AP_SHARED_USER.  That
is a mistake since pw now points to AP_SHARED_USER info instead of
to target_uname info.

Cheers,
Glenn

    /*
     * Error out if the target username is invalid.
     */
    if (strspn(target_uname, "1234567890") != strlen(target_uname)) {
        if ((pw = getpwnam(target_uname)) == NULL) {
            log_err("invalid target user name: (%s)\n", target_uname);
            exit(105);
        }
    }
    else {
        if ((pw = getpwuid(atoi(target_uname))) == NULL) {
            log_err("invalid target user id: (%s)\n", target_uname);
            exit(121);
        }
    }

[ Should copy information out from pw here ]

    if (strspn(AP_SHARED_USER, "1234567890") != strlen(AP_SHARED_USER)) {
        if ((spw = getpwnam(AP_SHARED_USER)) == NULL) {
            log_err("invalid target shared user name: (%s)\n", AP_SHARED_USER);
            exit(200);
        }
    }
    else {
        if ((spw = getpwuid(atoi(AP_SHARED_USER))) == NULL) {
            log_err("invalid target shared user id: (%s)\n", AP_SHARED_USER);
            exit(200);
        }
    }

[...]
[ Access to pw here is possibly incorrect;
  might contain AP_SHARED_USER info instead of target_uname ]

    /*
     * Save these for later since initgroups will hose the struct
     */
    uid = pw->pw_uid;
    actual_uname = strdup(pw->pw_name);
    target_homedir = strdup(pw->pw_dir);