Rework storing files thoroughly. This includes:

o Remove the race between stat(2) & fopen(3) when creating
  a unique file.

o Improve bound checking when generating a unique name from
  a given pathname.

o Ignore REST marker on APPE.  No RFC specifies this case,
  but the idea of resuming APPE's implies this.

o By default, deny upload resumes and appends by anonymous users.
  Previously these commands were translated to STOU silently,
  which led to broken files on server without any notification
  to the user.

o Add an option, -m, to allow anonymous users to modify
  existing files (e.g., to resume uploads) if filesystem
  permissions permit.

Portions obrainded from:	OpenBSD
MFC after:			3 weeks

2 files changed, 84 insertions, 24 deletions

diff --git a/libexec/ftpd/ftpd.8 b/libexec/ftpd/ftpd.8
index 4a44beb..80d95ec 100644
--- a/libexec/ftpd/ftpd.8
+++ b/libexec/ftpd/ftpd.8
@@ -40,7 +40,7 @@
 .Nd Internet File Transfer Protocol server
 .Sh SYNOPSIS
 .Nm
-.Op Fl 46AdDEMoOrRSUv
+.Op Fl 46AdDEmMoOrRSUv
 .Op Fl l Op Fl l
 .Op Fl a Ar address
 .Op Fl p Ar file
@@ -121,6 +121,11 @@ are not displayed by
 by default, and may have to be enabled in
 .Xr syslogd 8 Ns 's
 configuration file.
+.It Fl m
+Permit anonymous users to overwrite or modify
+existing files if allowed by filesystem permissions.
+By default, anonymous users cannot modify existing files;
+in particular, files to upload will be created under a unique name.
 .It Fl M
 Prevent anonymous users from creating directories.
 .It Fl o
diff --git a/libexec/ftpd/ftpd.c b/libexec/ftpd/ftpd.c
index d016d2e..5c68b84 100644
--- a/libexec/ftpd/ftpd.c
+++ b/libexec/ftpd/ftpd.c
@@ -139,6 +139,7 @@ int	noepsv=0;		/* EPSV command is disabled.	*/
 int	noretr=0;		/* RETR command is disabled.	*/
 int	noguestretr=0;		/* RETR command is disabled for anon users. */
 int	noguestmkd=0;		/* MKD command is disabled for anon users. */
+int	noguestmod=1;		/* anon users may not modify existing files. */
 
 static volatile sig_atomic_t recvurg;
 sig_atomic_t transflag;
@@ -242,7 +243,7 @@ static void	 dolog(struct sockaddr *);
 static char	*curdir(void);
 static void	 end_login(void);
 static FILE	*getdatasock(char *);
-static char	*gunique(char *);
+static int	 guniquefd(char *, char **);
 static void	 lostconn(int);
 static void	 sigquit(int);
 static int	 receive_data(FILE *, FILE *);
@@ -294,7 +295,7 @@ main(int argc, char *argv[], char **envp)
 #endif /* OLD_SETPROCTITLE */
 
 
-	while ((ch = getopt(argc, argv, "46a:AdDElMoOp:rRSt:T:u:Uv")) != -1) {
+	while ((ch = getopt(argc, argv, "46a:AdDElmMoOp:rRSt:T:u:Uv")) != -1) {
 		switch (ch) {
 		case '4':
 			enable_v4 = 1;
@@ -330,6 +331,10 @@ main(int argc, char *argv[], char **envp)
 			logging++;	/* > 1 == extra logging */
 			break;
 
+		case 'm':
+			noguestmod = 0;
+			break;
+
 		case 'M':
 			noguestmkd = 1;
 			break;
@@ -1587,24 +1592,45 @@ done:
 void
 store(char *name, char *mode, int unique)
 {
+	int fd;
 	FILE *fout, *din;
 	struct stat st;
 	int (*closefunc)(FILE *);
 
-	if ((unique || guest) && stat(name, &st) == 0 &&
-	    (name = gunique(name)) == NULL) {
-		LOGCMD(*mode == 'w' ? "put" : "append", name);
-		return;
+	if (*mode == 'a') {		/* APPE */
+		if (unique) {
+			/* Programming error */
+			syslog(LOG_ERR, "Internal: unique flag to APPE");
+			unique = 0;
+		}
+		if (guest && noguestmod) {
+			reply(550, "Appending to existing file denied");
+			goto err;
+		}
+		restart_point = 0;	/* not affected by preceding REST */
+	}
+	if (unique)			/* STOU overrides REST */
+		restart_point = 0;
+	if (guest && noguestmod) {
+		if (restart_point) {	/* guest STOR w/REST */
+			reply(550, "Modifying existing file denied");
+			goto err;
+		} else			/* treat guest STOR as STOU */
+			unique = 1;
 	}
 
 	if (restart_point)
-		mode = "r+";
-	fout = fopen(name, mode);
+		mode = "r+";	/* so ASCII manual seek can work */
+	if (unique) {
+		if ((fd = guniquefd(name, &name)) < 0)
+			goto err;
+		fout = fdopen(fd, mode);
+	} else
+		fout = fopen(name, mode);
 	closefunc = fclose;
 	if (fout == NULL) {
 		perror_reply(553, name);
-		LOGCMD(*mode == 'w' ? "put" : "append", name);
-		return;
+		goto err;
 	}
 	byte_count = -1;
 	if (restart_point) {
@@ -1652,6 +1678,10 @@ store(char *name, char *mode, int unique)
 done:
 	LOGBYTES(*mode == 'w' ? "put" : "append", name, byte_count);
 	(*closefunc)(fout);
+	return;
+err:
+	LOGCMD(*mode == 'a' ? "append" : "put" , name);
+	return;
 }
 
 static FILE *
@@ -2690,38 +2720,63 @@ pasv_error:
 }
 
 /*
- * Generate unique name for file with basename "local".
- * The file named "local" is already known to exist.
+ * Generate unique name for file with basename "local"
+ * and open the file in order to avoid possible races.
+ * Try "local" first, then "local.1", "local.2" etc, up to "local.99".
+ * Return descriptor to the file, set "name" to its name.
+ *
  * Generates failure reply on error.
  */
-static char *
-gunique(char *local)
+static int
+guniquefd(char *local, char **name)
 {
 	static char new[MAXPATHLEN];
 	struct stat st;
-	int count;
 	char *cp;
+	int count;
+	int fd;
 
 	cp = strrchr(local, '/');
 	if (cp)
 		*cp = '\0';
 	if (stat(cp ? local : ".", &st) < 0) {
 		perror_reply(553, cp ? local : ".");
-		return ((char *) 0);
+		return (-1);
 	}
-	if (cp)
+	if (cp) {
+		/*
+		 * Let not overwrite dirname with counter suffix.
+		 * -4 is for /nn\0
+		 * In this extreme case dot won't be put in front of suffix.
+		 */
+		if (strlen(local) > sizeof(new) - 4) {
+			reply(553, "Pathname too long");
+			return (-1);
+		}
 		*cp = '/';
+	}
 	/* -4 is for the .nn<null> we put on the end below */
 	(void) snprintf(new, sizeof(new) - 4, "%s", local);
 	cp = new + strlen(new);
-	*cp++ = '.';
-	for (count = 1; count < 100; count++) {
-		(void)sprintf(cp, "%d", count);
-		if (stat(new, &st) < 0)
-			return (new);
+	/* 
+	 * Don't generate dotfile unless requested explicitly.
+	 * This covers the case when basename gets truncated off
+	 * by buffer size.
+	 */
+	if (cp > new && cp[-1] != '/')
+		*cp++ = '.';
+	for (count = 0; count < 100; count++) {
+		/* At count 0 try unmodified name */
+		if (count)
+			(void)sprintf(cp, "%d", count);
+		if ((fd = open(count ? new : local,
+		    O_RDWR | O_CREAT | O_EXCL, 0666)) >= 0) {
+			*name = count ? new : local;
+			return (fd);
+		}
 	}
 	reply(452, "Unique file name cannot be created.");
-	return (NULL);
+	return (-1);
 }
 
 /*