tftpd: try to handle duplicate WRQ packets

Duplicate WRQ packets can really hurt, since they end up accessing the
same file.  This attempts to lock the file, which should work for the
case where a correctly implemented TFTP stack uses the same session ID
(port number) for each retry; in any other case they look like
multiple sessions to the same file and it is a crapshoot if we end up
with the correct one.

Signed-off-by: H. Peter Anvin <hpa@zytor.com>

2 files changed, 41 insertions, 9 deletions

diff --git a/configure.in b/configure.in
index ca21af7..7ab7c5a 100644
--- a/configure.in
+++ b/configure.in
@@ -46,6 +46,7 @@ AC_CHECK_HEADERS(strings.h)
 AC_CHECK_HEADERS(sysexits.h)
 AC_CHECK_HEADERS(time.h)
 AC_CHECK_HEADERS(unistd.h)
+AC_CHECK_HEADERS(sys/file.h)
 AC_CHECK_HEADERS(sys/filio.h)
 AC_CHECK_HEADERS(sys/stat.h)
 AC_CHECK_HEADERS(sys/time.h)
@@ -140,6 +141,11 @@ PA_HEADER_DEFINES(fcntl.h, int, O_NONBLOCK)
 PA_HEADER_DEFINES(fcntl.h, int, O_BINARY)
 PA_HEADER_DEFINES(fcntl.h, int, O_TEXT)
 
+PA_HEADER_DEFINES(fcntl.h, int, F_SETLK)
+
+PA_HEADER_DEFINES(sys/file.h, int, LOCK_SH)
+PA_HEADER_DEFINES(sys/file.h, int, LOCK_EX)
+
 AH_TEMPLATE([HAVE_SIGSETJMP],
 [Define if we have sigsetjmp, siglongjmp and sigjmp_buf.])
 PA_SIGSETJMP([AC_DEFINE(HAVE_SIGSETJMP)])
diff --git a/tftpd/tftpd.c b/tftpd/tftpd.c
index c122d61..4c4d4ed 100644
--- a/tftpd/tftpd.c
+++ b/tftpd/tftpd.c
@@ -179,6 +179,26 @@ static struct rule *read_remap_rules(const char *file)
 }
 #endif
 
+/*
+ * Rules for locking files; return 0 on success, -1 on failure
+ */
+static int lock_file(int fd, int lock_write)
+{
+#if defined(HAVE_FCNTL) && defined(HAVE_F_SETLK_DEFINITION)
+  struct flock fl;
+
+  fl.l_type   = lock_write ? F_WRLCK : F_RDLCK;
+  fl.l_whence = SEEK_SET;
+  fl.l_start  = 0;
+  fl.l_len    = 0;		/* Whole file */
+  return fcntl(fd, F_SETLK, &fl);
+#elif defined(HAVE_LOCK_SH_DEFINITION)
+  return flock(fd, lock_write ? LOCK_EX|LOCK_NB : LOCK_SH|LOCK_NB);
+#else
+  return 0;			/* Hope & pray... */
+#endif
+}
+
 static void set_socket_nonblock(int fd, int flag)
 {
     int err;
@@ -1463,11 +1483,13 @@ static int validate_access(char *filename, int mode,
      * We use different a different permissions scheme if `cancreate' is
      * set.
      */
-    wmode = O_WRONLY |
-        (cancreate ? O_CREAT : 0) |
-        (unixperms ? O_TRUNC : 0) | (pf->f_convert ? O_TEXT : O_BINARY);
+    wmode = O_WRONLY | (cancreate ? O_CREAT : 0) | (pf->f_convert ? O_TEXT : O_BINARY);
     rmode = O_RDONLY | (pf->f_convert ? O_TEXT : O_BINARY);
 
+#ifndef HAVE_FTRUNCATE
+    wmode |= O_TRUNC;		/* This really sucks on a dupe */
+#endif
+
     fd = open(filename, mode == RRQ ? rmode : wmode, 0666);
     if (fd < 0) {
         switch (errno) {
@@ -1486,6 +1508,10 @@ static int validate_access(char *filename, int mode,
     if (fstat(fd, &stbuf) < 0)
         exit(EX_OSERR);         /* This shouldn't happen */
 
+    /* A duplicate RRQ or (worse!) WRQ packet could really cause havoc... */
+    if (lock_file(fd, mode != RRQ))
+	exit(0);
+
     if (mode == RRQ) {
         if (!unixperms && (stbuf.st_mode & (S_IREAD >> 6)) == 0) {
             *errmsg = "File must have global read permissions";
@@ -1500,15 +1526,15 @@ static int validate_access(char *filename, int mode,
                 *errmsg = "File must have global write permissions";
                 return (EACCESS);
             }
+        }
 
-            /* We didn't get to truncate the file at open() time */
 #ifdef HAVE_FTRUNCATE
-            if (ftruncate(fd, (off_t) 0)) {
-                *errmsg = "Cannot reset file size";
-                return (EACCESS);
-            }
+	/* We didn't get to truncate the file at open() time */
+	if (ftruncate(fd, (off_t) 0)) {
+	  *errmsg = "Cannot reset file size";
+	  return (EACCESS);
+	}
 #endif
-        }
         tsize = 0;
         tsize_ok = 1;
     }