Simplify globbing of ftpd(8)s list and nlst commands.

Also avoid command option injection for ls(1).

OK martijn@

3 files changed, 25 insertions, 34 deletions

diff --git a/libexec/ftpd/extern.h b/libexec/ftpd/extern.h
index a2e95cdd34d..3587af7dc28 100644
--- a/libexec/ftpd/extern.h
+++ b/libexec/ftpd/extern.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: extern.h,v 1.20 2019/05/08 23:56:48 tedu Exp $	*/
+/*	$OpenBSD: extern.h,v 1.21 2020/01/15 22:06:59 jan Exp $	*/
 /*	$NetBSD: extern.h,v 1.2 1995/04/11 02:44:49 cgd Exp $	*/
 
 /*
@@ -68,7 +68,7 @@ void	delete(char *);
 void	dologout(int);
 void	fatal(char *);
 int	ftpd_pclose(FILE *, pid_t);
-FILE   *ftpd_ls(char *, char *, pid_t *);
+FILE   *ftpd_ls(const char *, pid_t *);
 int     get_line(char *, int, FILE *);
 void	ftpdlogwtmp(char *, char *, char *);
 void	lreply(int, const char *, ...);
diff --git a/libexec/ftpd/ftpd.c b/libexec/ftpd/ftpd.c
index 243881be9c3..7fba613baf9 100644
--- a/libexec/ftpd/ftpd.c
+++ b/libexec/ftpd/ftpd.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ftpd.c,v 1.228 2019/07/03 03:24:04 deraadt Exp $	*/
+/*	$OpenBSD: ftpd.c,v 1.229 2020/01/15 22:06:59 jan Exp $	*/
 /*	$NetBSD: ftpd.c,v 1.15 1995/06/03 22:46:47 mycroft Exp $	*/
 
 /*
@@ -1124,7 +1124,7 @@ retrieve(enum ret_cmd cmd, char *name)
 		fin = fopen(name, "r");
 		st.st_size = 0;
 	} else {
-		fin = ftpd_ls("-lgA", name, &pid);
+		fin = ftpd_ls(name, &pid);
 		st.st_size = -1;
 		st.st_blksize = BUFSIZ;
 	}
@@ -1730,7 +1730,7 @@ statfilecmd(char *filename)
 	int c;
 	int atstart;
 	pid_t pid;
-	fin = ftpd_ls("-lgA", filename, &pid);
+	fin = ftpd_ls(filename, &pid);
 	if (fin == NULL) {
 		reply(451, "Local resource failure");
 		return;
diff --git a/libexec/ftpd/popen.c b/libexec/ftpd/popen.c
index cfbb9d81674..9e7bb4c6388 100644
--- a/libexec/ftpd/popen.c
+++ b/libexec/ftpd/popen.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: popen.c,v 1.28 2019/06/28 13:32:53 deraadt Exp $	*/
+/*	$OpenBSD: popen.c,v 1.29 2020/01/15 22:06:59 jan Exp $	*/
 /*	$NetBSD: popen.c,v 1.5 1995/04/11 02:45:00 cgd Exp $	*/
 
 /*
@@ -39,6 +39,7 @@
 
 #include <errno.h>
 #include <glob.h>
+#include <limits.h>
 #include <signal.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -56,55 +57,44 @@
  * may create a pipe to a hidden program as a side effect of a list or dir
  * command.
  */
-#define MAX_ARGV	100
-#define MAX_GARGV	1000
 
 FILE *
-ftpd_ls(char *arg, char *path, pid_t *pidptr)
+ftpd_ls(const char *path, pid_t *pidptr)
 {
 	char *cp;
 	FILE *iop;
-	int argc = 0, gargc, pdes[2];
+	int argc = 0, pdes[2];
 	pid_t pid;
-	char **pop, *argv[MAX_ARGV], *gargv[MAX_GARGV];
+	char **pop, *argv[_POSIX_ARG_MAX];
 
 	if (pipe(pdes) == -1)
 		return (NULL);
 
 	/* break up string into pieces */
 	argv[argc++] = "/bin/ls";
-	if (arg != NULL)
-		argv[argc++] = arg;
-	if (path != NULL)
-		argv[argc++] = path;
-	argv[argc] = NULL;
-	argv[MAX_ARGV-1] = NULL;
+	argv[argc++] = "-lgA";
+	argv[argc++] = "--";
 
-	/* glob each piece */
-	gargv[0] = argv[0];
-	for (gargc = argc = 1; argv[argc]; argc++) {
+	/* glob that path */
+	if (path != NULL) {
 		glob_t gl;
 
 		memset(&gl, 0, sizeof(gl));
-		if (glob(argv[argc],
+		if (glob(path,
 		    GLOB_BRACE|GLOB_NOCHECK|GLOB_QUOTE|GLOB_TILDE|GLOB_LIMIT,
 		    NULL, &gl)) {
-			if (gargc < MAX_GARGV-1) {
-				gargv[gargc++] = strdup(argv[argc]);
-				if (gargv[gargc -1] == NULL)
-					fatal ("Out of memory.");
-			}
-
+			fatal ("Glob error.");
 		} else if (gl.gl_pathc > 0) {
-			for (pop = gl.gl_pathv; *pop && gargc < MAX_GARGV-1; pop++) {
-				gargv[gargc++] = strdup(*pop);
-				if (gargv[gargc - 1] == NULL)
+			for (pop = gl.gl_pathv; *pop && argc < _POSIX_ARG_MAX-1;
+			    pop++) {
+				argv[argc++] = strdup(*pop);
+				if (argv[argc - 1] == NULL)
 					fatal ("Out of memory.");
 			}
 		}
 		globfree(&gl);
 	}
-	gargv[gargc] = NULL;
+	argv[argc] = NULL;
 
 	iop = NULL;
 
@@ -128,15 +118,16 @@ ftpd_ls(char *arg, char *path, pid_t *pidptr)
 
 		/* reset getopt for ls_main */
 		optreset = optind = 1;
-		exit(ls_main(gargc, gargv));
+		exit(ls_main(argc, argv));
 	}
 	/* parent; assume fdopen can't fail...  */
 	iop = fdopen(pdes[0], "r");
 	(void)close(pdes[1]);
 	*pidptr = pid;
 
-pfree:	for (argc = 1; gargv[argc] != NULL; argc++)
-		free(gargv[argc]);
+ pfree:
+	for (argc = 3; argv[argc] != NULL; argc++)
+		free(argv[argc]);
 
 	return (iop);
 }