Subject: tftpd oddity, PR 4637, code fix enclosed
To: None <current-users@netbsd.org, gnats-bug@netbsd.org>
From: Alexis Rosen <alexis@panix.com>
List: current-users
Date: 01/29/1999 16:38:04
While trying to move a tftp server to NetBSD from SunOS, I ran into a problem:
My ciscos could download files but not upload.
Reading the source made it clear: NetBSD's tftpd demands (in validate_access())
that "full path name must be given as we have no login directory", according
to a comment (line 459 of tftpd.c).
This turns out to be PR 4637. The fix recommended there is to search in all
of the paths listed in the invocation. I disagree- it should simply try to
use / or the / of the chroot as a default. The other alternative violates
the Principle of Least Surprise.
Here is my patch to fix the problem. If nobody objects to this, I'll get it
committed in a few days.
BTW, what's the proper protocol for doing this? I'm copying this to both
current-users (to see if anyone doesn't like this) and to gnats-bug to
update the PR. Is this bad?
/a
---
Alexis Rosen Owner/Sysadmin,
PANIX Public Access Unix & Internet, NYC.
alexis@panix.com
--- tftpd.c Sun Sep 20 07:10:00 1998
+++ /var/home/alexis/tftpd.c Fri Jan 29 12:26:51 1999
@@ -456,8 +456,6 @@
* If we were invoked with arguments
* from inetd then the file must also be
* in one of the given directory prefixes.
- * Note also, full path name must be
- * given as we have no login directory.
*/
int
validate_access(filep, mode)
@@ -470,6 +468,12 @@
static char pathname[MAXPATHLEN];
char *filename = *filep;
+ if ((mode == WRQ) && (*filename != '/')) {
+ /* For writes only, assume relative filenames are in "/". */
+ snprintf(pathname, sizeof pathname, "/%s", filename);
+ *filep = filename = pathname;
+ }
+
/*
* Prevent tricksters from getting around the directory restrictions
*/
@@ -508,11 +512,13 @@
/*
* Relative file name: search the approved locations for it.
- * Don't allow write requests or ones that avoid directory
- * restrictions.
+ * Must be a read request (writes get "/" prepended to
+ * relative pathnames and are always handled by the other
+ * branch of this if). So we can still test against filename
+ * and use pathname for scratch.
*/
- if (mode != RRQ || !strncmp(filename, "../", 3))
+ if (!strncmp(filename, "../", 3))
return (EACCESS);
/*