Subject: Re: wu-ftpd beta 13 Upload Ownership/Permissions Bug

From: Ian Willis <[email-masked]>
Date: Fri, 20 Jun 97 16:56:51 BST

Your change introduces a nice feature, it is now possible to specify
a home directory for the anonymous user under the anonymous root
(e.g. with a directory of /home/ftp/./pub).

Your change can be simplified slightly, there is no need to put the
"if (anonymous || guest) {" test  around the strstr(), we are already
inside an "if (anonymous || guest) {" test.

Ian Willis
SCO Internet Engineering Group, Watford, UK

| Michael Brennen ([email-masked])
| Re: wu-ftpd beta 13 Upload Ownership/Permissions Bug
| Fri, 20 Jun 1997 08:09:51 -0500 (CDT)
| 
| On Fri, 20 Jun 1997, Ian Willis wrote:
| 
| > The code in question has not changed between Beta 11 and Beta 13, why
| > would you have /./ in the home directory field for the anonymous user,
| > I thought this was only meaningful with guest accounts which only apply
| > to real users ?
| 
| After some sleep, it looks like I recently went through and added the /./
| to the end of the anon ftp path in /etc/passwd. I must have done this
| while looking at other things. I certainly had no idea that it would break
| the upload directive code.
| 
| The argument could be made that the /./ should never be on the anon ftp
| path. Given the unexpected consequences of placing it there, and that
| adding the patch does not alter functionality if /./ is not there, I would
| still argue that the source change should be made in the eventuality that
| someone puts /./ on their anon ftp path.
| 
| anonymous is a chrooted account, and it would be easy to think you needed
| the /./. I've worked with this stuff for a long time, and perhaps
| arguably should know better. But, in a moment of inattention, while not
| thinking of the full config options of wu-ftpd, I added /./ and
| unexpectedly changed the behaviour of the daemon for the worse. I guess I
| would like to see that hole closed.
| 
| -- Michael

---

From: Michael Brennen <[email-masked]>
Date: Fri, 20 Jun 1997 11:25:40 -0500 (CDT)

> (e.g. with a directory of /home/ftp/./pub).

I saw that too; could be a nice side effect the more I think about it. 

> inside an "if (anonymous || guest) {" test.

I know; sleep does wonderful things for peripheral vision. :) I saw that
the anonymous || guest check was already in a qualified anonymous || guest
block this morning.  A better patch against the original source is below; 
reverse the other one first (patch -R) before applying this one.  This has
tested out okay here.

   -- Michael

--- ftpd.c.orig	Wed May 21 09:29:17 1997
+++ ftpd.c	Fri Jun 20 11:19:01 1997
@@ -1550,6 +1550,8 @@
     expand_id();
 
     if (anonymous || guest) {
+        char *sp;
+
         /* We MUST do a chdir() after the chroot. Otherwise the old current
          * directory will be accessible as "." outside the new root! */
 #ifdef VIRTUAL
@@ -1559,28 +1561,19 @@
             pw->pw_dir = sgetsave(virtual_root);
         }
 #endif 
-        if (anonymous) {
+        /* determine root and home directory */
+
+        if ((sp = strstr(pw->pw_dir, "/./")) == NULL) {
             if (chroot(pw->pw_dir) < 0 || chdir("/") < 0) {
                 reply(550, "Can't set guest privileges.");
                 goto bad;
             }
-        } else if (guest) {
-            char *sp;
-
-            /* determine root and home directory */
+        } else {
+            *sp++ = '\0';
 
-            if ((sp = strstr(pw->pw_dir, "/./")) == NULL) {
-                if (chroot(pw->pw_dir) < 0 || chdir("/") < 0) {
-                    reply(550, "Can't set guest privileges.");
-                    goto bad;
-                }
-            } else {
-                *sp++ = '\0';
-
-                if (chroot(pw->pw_dir) < 0 || chdir(++sp) < 0) {
-                    reply(550, "Can't set guest privileges.");
-                    goto bad;
-                }
+            if (chroot(pw->pw_dir) < 0 || chdir(++sp) < 0) {
+                reply(550, "Can't set guest privileges.");
+                goto bad;
             }
         }
     }