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);
 
 		/*