httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Glenn <>
Subject Re: suexec enhancement
Date Sun, 08 Feb 2004 02:02:31 GMT
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.


     * 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);
    else {
        if ((pw = getpwuid(atoi(target_uname))) == NULL) {
            log_err("invalid target user id: (%s)\n", target_uname);

[ 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);
    else {
        if ((spw = getpwuid(atoi(AP_SHARED_USER))) == NULL) {
            log_err("invalid target shared user id: (%s)\n", AP_SHARED_USER);

[ 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);

View raw message