Subject: Directory owner/group in upload

From: Alvaro Martinez Echevarria <[email-masked]>
Date: Fri, 31 Jan 1997 04:14:13 +0100 (MET)

Hi
I have heard from a friend (he is subscribed to this list, but I'm not)
that there has been a few messages lately related to the problems about
creating subdirectories under an upload directory. Right now, it doesn't
matter if you specify owner and group in the ftpaccess file, with
subdirectory creation it won't work (at least with the last version I
have found, 2.4.2-beta-12).
I suffered this problem almost a year ago, because I was programming
some custom stuff on my linux box, and I needed the subdirectories to be
created correctly. So I made a dirty patch to get it solved (the patch
I include in the email). At that time I was using wu-ftpd version 2.4,
so I have added a couple of lines to make it work under 2.4.2-beta-12.
This patch has worked (with 2.4) for almost a year with no problems
at all.  Under 2.4.2-beta-12 it apparently works, but I am not sure
(maybe it won't compile on some platforms, I just have access to sunOS
and linux). Anyway, I hope this helps to solve the problem.
Regards.
PS: by the way, as I am not on the list, would you people on the
wu-ftpd-list send me copy of the responses? (if I get any :-) )
Thanks

.------------------------------------------------------------------.
|   Alvaro Martínez Echevarría   |   LANDER SISTEMAS               |
|        [email-masked]        |   Pº Castellana, 121, 2ºC dcha  |
`--------------------------------|   28046 Madrid, SPAIN           |
                                 |   Tel: +34-1-5562883            |
                                 |   Fax: +34-1-5563001            |
                                 `---------------------------------'

--- src/ftpd.c.OLD	Mon Jan 20 06:05:11 1997
+++ src/ftpd.c	Fri Jan 31 03:38:21 1997
@@ -2938,13 +2938,97 @@
     if ( (fn_check(name)) <= 0 )
         return;
 
-    if (mkdir(name, 0777) < 0) {
-        if (errno == EEXIST)
-            perror_reply(553, name);
-        else
-            perror_reply(550, name);
-	return;
+    /*
+     * Fri Jan 31 03:32:25 MET 1997 [email-masked]
+     * If a valid uid/gid pair is given, we use it.
+     */
+    if (valid > 0) {
+        pid_t oldeuid;
+        gid_t oldegid;
+        char *c, *parent;
+        int perm,olderrno;
+
+        /*
+         * We create the subdirectory this ugly way to avoid a
+         * race condition with chown, and hence we must check
+         * file permissions before creating it. Anyway, probably
+         * this isn't secure enough.
+         */
+        oldeuid=geteuid();
+        oldegid=getegid();
+        if (debug) {
+            syslog(LOG_INFO,"step 1: uid=%d, euid=%d, gid=%d, egid=%d",getuid(),geteuid(),getgid(),getegid());
+        }
+
+        (void) seteuid((uid_t) 0);
+        (void) setreuid(oldeuid, (uid_t)0);
+        (void) setregid(oldegid, (gid_t)0);
+        if (debug) {
+            syslog(LOG_INFO,"step 2: uid=%d, euid=%d, gid=%d, egid=%d",getuid(),geteuid(),getgid(),getegid());
+        }
+
+        c=strrchr(name,'/');
+        if (c==NULL) {
+            parent=".";
+        } else {
+            *c=0;
+            parent=name;
+            if (*parent==0) {
+                parent="/";
+            }
+        }
+        if (debug) {
+            syslog(LOG_INFO,"testing access");
+        }
+        perm=access(parent,W_OK);
+        if (c!=NULL) *c='/';
+
+        (void) setregid((gid_t) 0, (gid_t)gid);
+        (void) setreuid((uid_t) 0, (uid_t)uid);
+        if (debug) {
+            syslog(LOG_INFO,"step 3: uid=%d, euid=%d, gid=%d, egid=%d",getuid(),geteuid(),getgid(),getegid());
+        }
+        if (debug) {
+            syslog(LOG_INFO,"trying to mkdir()");
+        }
+        if (perm || mkdir(name, 0777) < 0) {
+            olderrno=errno;
+            (void) seteuid((uid_t) 0);
+            (void) setregid((gid_t) 0, oldegid);
+            (void) setreuid((uid_t) 0, oldeuid);
+            if (debug) {
+                if (perm) {
+                    syslog(LOG_INFO,"can't mkdir(): access denied");
+                } else {
+                    syslog(LOG_INFO,"can't mkdir(): error");
+                }
+                syslog(LOG_INFO,"step 4: uid=%d, euid=%d, gid=%d, egid=%d",getuid(),geteuid(),getgid(),getegid());
+            }
+            if (olderrno == EEXIST)
+	        perror_reply(553, name);
+	    else
+	        perror_reply(550, name);
+            return;
+        }
+        (void) seteuid((uid_t) 0);
+        (void) setregid((gid_t) 0, oldegid);
+        (void) setreuid((uid_t) 0, oldeuid);
+        if (debug) {
+            syslog(LOG_INFO,"step 4: uid=%d, euid=%d, gid=%d, egid=%d",getuid(),geteuid(),getgid(),getegid());
+        }
+    } else {
+        if (mkdir(name, 0777) < 0) {
+            if (errno == EEXIST)
+	        perror_reply(553, name);
+	    else
+	        perror_reply(550, name);
+            return;
+        }
     }
+    /*
+     * End of ugly modification
+     */
+
     reply(257, "MKD command successful.");
 }

---

From: Mark Galbraith <[email-masked]>
Date: Thu, 30 Jan 1997 20:14:21 -0800 (PST)

Actually, I just completed a similar change to my 2.4(6) code base
here.  My changes are a bit smaller, but they don't do many of the race
condition checks you are doing.

I'd be interested in knowing what race conditions you might see
resulting from my patch (attached below).  I'd guess you're worried
about the name of the directory changing between the mkdir() call and
the chown() call.  If that happens, the message is the mkdir succeeded
but the chown failed.  How serious is this?  Should I be worried?

-- 
Mark Galbraith                   Member of The HTML Writers Guild
Engineer, Internet Services	              http://www.hwg.org/
Triad Systems Corporation
(PGP Fingerprint=1CB9 7481 AD5C 5709  690B AC09 7F65 D6F6)

------------ Patch to chown directories to the proper owner ------------

*** ftpd.c-	Mon Jan  6 07:27:09 1997
--- ftpd.c	Thu Jan 30 19:53:27 1997
***************
*** 2252,2257 ****
--- 2252,2258 ----
  {
  	uid_t uid;
  	gid_t gid;
+ 	uid_t oldid;
  	int   valid = 0;
  
      /*
***************
*** 2270,2275 ****
--- 2271,2294 ----
          perror_reply(550, name);
  	return;
      }
+     /* PATCH: Modification to set the user and group on the directory just
+        made.  This change by Mark Galbraith of Triad Systems.
+        <[email-masked]>
+ 
+        This code is largely lifted from the file upload section. */
+ 
+     /* if we have a uid and gid, then use them. */
+ 
+     if (valid > 0) {
+         oldid = geteuid();
+         (void) seteuid((uid_t) 0);
+         if ((chown(name, uid, gid)) < 0) {
+             (void) seteuid(oldid);
+             reply(257, "MKD command successful.  chown() on directory failed.");
+             return;
+         }
+         (void) seteuid(oldid);
+     }

---

From: Alvaro Martinez Echevarria <[email-masked]>
Date: Fri, 31 Jan 1997 06:23:04 +0100 (MET)

On Thu, 30 Jan 1997, Mark Galbraith wrote:

> Date: Thu, 30 Jan 1997 20:14:21 -0800 (PST)
> From: Mark Galbraith <[email-masked]>
> To: [email-masked]
> Subject: Re: Directory owner/group in upload
> Resent-Date: Fri, 31 Jan 1997 05:59:20 +0100 (MET)
> Resent-From: Roberto Lumbreras Pastor <[email-masked]>
> Resent-To: Alvaro Martínez Echevarría <[email-masked]>
> 

Actually the problem isn't that the chmod() could fail, but probably
something more serious. Consider the following situation:

wu.ftpd                     evil user
-------                     ---------
                            mkdir ./foo
        <--------------------
mkdir(./foo)
chown(./foo,XXX)
         -------------------->
                            mkdir ./foo/passwd
        <--------------------
mkdir(./foo/passwd)         
         -------------------->
                            # here we go, in a shell he/she does:
                            mv ./foo ./foo.OLD
                            ln -s /etc ./foo
        <--------------------
chown(./foo/passwd,XXX)
         -------------------->
                            vi /etc/passwd

This is just a silly example, because it would require the user to have
both shell access in the machine and write permission in the upload
directory.  But I'm pretty sure there's a lot of people out there
willing to find examples better than mine }:-)
Regards

---

From: Mark Galbraith <[email-masked]>
Date: Fri, 31 Jan 1997 17:40:15 -0800 (PST)

>>>>> "Alvaro" == Alvaro Martinez Echevarria <[email-masked]> writes:

Alvaro> Actually the problem isn't that the chmod() could fail, but probably
Alvaro> something more serious. Consider the following situation:

Alvaro> wu.ftpd                     evil user
Alvaro> -------                     ---------
Alvaro>                             mkdir ./foo
Alvaro>         <--------------------
Alvaro> mkdir(./foo)
Alvaro> chown(./foo,XXX)
Alvaro>		--------------------> 
Alvaro> mkdir ./foo/passwd
Alvaro>         <--------------------
Alvaro> mkdir(./foo/passwd)         
Alvaro>		-------------------->   [[ BTW, I think you probably
Alvaro>					   meant to omit this. --MGG ]]
Alvaro>                             # here we go, in a shell he/she does:
Alvaro>                             mv ./foo ./foo.OLD
Alvaro>                             ln -s /etc ./foo
Alvaro>         <--------------------   [[ BTW, I think you probably
Alvaro>					   meant to omit this. --MGG ]]
Alvaro> chown(./foo/passwd,XXX)
Alvaro>		--------------------> 
Alvaro>                             vi /etc/passwd

Alvaro> This is just a silly example, because it would require the user to have
Alvaro> both shell access in the machine and write permission in the upload
Alvaro> directory.  But I'm pretty sure there's a lot of people out there
Alvaro> willing to find examples better than mine }:-)
Alvaro> Regards

I think I understand this, but I'm still not sure I see the threat.
Perhaps I'm being dense, or maybe just not paranoid enough.  ;-)

If the user is chroot'd into their own area, and they edit the
/etc/passwd file (which is a copy containing only the ID's necessary
for anonymous FTP), what have they accomplished.  My understanding is
that the passwd file in ~ftp/etc/ is not used by any program other than
~ftp/bin/ls.  Editing ~ftp/etc/passwd shouldn't have any more effect
than making the 'dir' command return strange usernames.

Is the ~ftp/etc/passwd file used for more than just the output of 'ls'?
If so, for what?

Of course, you could be using ~ftp/etc/passwd as an example, meaning
that any file in the ftp area is vulnerable.  On this, I agree.  The
moment you allow uploads of any kind, you have created a vulnerability.
The only questions now is how vulnerable do you want to allow your system
to be?  And is there a way to reduce that vulnerability while still
providing the functionality necessary?