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

From: Ian Willis <[email-masked]>
Date: Fri, 20 Jun 97 11:22:42 BST

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 ?

Ian Willis
SCO Internet Engineering Group, Watford, UK

| Michael Brennen ([email-masked])
| wu-ftpd beta 13 Upload Ownership/Permissions Bug
| Thu, 19 Jun 1997 23:55:52 -0500 (CDT)
| 
| There is a potentially serious bug in ftpd.c in wu-ftpd beta 13. I have
| no idea if it exists in previous betas. I don't think this was a problem
| in beta 11, but I've not kept any older source. If you are not running
| beta 13, check this against your source.
| 
| During anonymous login, the /./ is not clipped off the struct passwd
| pw->pw_dir field that is saved as the chroot directory in ftpd.c.
| 
| Because the /./ is still on the end of the pw->pw_dir field, upl_check()
| 'upload' directive processing will fail in extensions.c because the file
| name paths will not compare. It is highly unlikely that the upload
| directive root path would also have the /./ on the end.
| 
| If upload directive processing fails for the anonymous user, sites that
| depend on upload directives to properly set incoming file permissions
| could find their site security compromised.
| 
| For example, it is fairly common to set incoming files to not be readable
| to the anonymous user; this prevents files from being traded through an
| anonymous site without the permission of the owner. With this bug, all
| uploaded files are owned by the anonymous user, with default permissions
| set by CMASK.
| 
| The patch below is my version of a fix. In testing so far it has fixed
| the problem. It may apply with an offset of 1; I'm not sure my ftpd.c
| source is pristine. The patch has been sent to academ.com.
| 
| -- Michael
| 
| --- ftpd.c.orig Thu Jun 19 22:59:21 1997
| +++ ftpd.c Thu Jun 19 23:01:26 1997
| @@ -1560,12 +1560,7 @@
| pw->pw_dir = sgetsave(virtual_root);
| }
| #endif
| - if (anonymous) {
| - if (chroot(pw->pw_dir) < 0 || chdir("/") < 0) {
| - reply(550, "Can't set guest privileges.");
| - goto bad;
| - }
| - } else if (guest) {
| + if (anonymous || guest) {
| char *sp;
| 
| /* determine root and home directory */

---

From: Michael Brennen <[email-masked]>
Date: Fri, 20 Jun 1997 08:09:51 -0500 (CDT)

> I thought this was only meaningful with guest accounts which only apply

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