progress: fix gzip -l injection and EINTR wait loop

Avoid popen()/shell when running gzip -l by forking gzip with argv, and handle EINTR correctly in the wait loop.

Add an ATF regression test for -z with metacharacters in filenames.

AI-Assisted-by: GPT-5.2 (Codex CLI)
Signed-off-by: Lucas Holt <luke@foolishgames.com>

Author: laffer1

usr.bin/progress/Makefile

@@ -1,6 +1,7 @@
 # $MidnightBSD: src/usr.bin/progress/Makefile,v 1.2 2012/12/02 06:26:32 laffer1 Exp $
 # $NetBSD: Makefile,v 1.2 2003/01/22 02:56:30 lukem Exp $
 
+.include <src.opts.mk>
 .include <bsd.own.mk>
 
 PROG=	progress
@@ -13,4 +14,9 @@ CFLAGS+= -I${.CURDIR}/../ftp
 
 .PATH:  ${.CURDIR}/../../contrib/tnftp/src
 
+.if ${MK_TESTS} != "no"
+HAS_TESTS=
+SUBDIR+= tests
+.endif
+
 .include <bsd.prog.mk>

usr.bin/progress/progress.c

@@ -60,6 +60,7 @@ __MBSDID("$MidnightBSD: src/usr.bin/progress/progress.c,v 1.4 2012/12/31 17:14:3
 
 static void broken_pipe(int unused);
 static void usage(void);
+static off_t gzip_uncompressed_size(const char *);
 
 long long
 strsuftoll(const char *desc, const char *val,
@@ -157,26 +158,7 @@ main(int argc, char *argv[])
 
 	/* gzip -l the file if we have the name and -z is given */
 	if (zflag && !lflag && infile != NULL) {
-		FILE *gzipsizepipe;
-		char buf[256], *cp, *cmd;
-
-		/*
-		 * Read second word of last line of gzip -l output. Looks like:
-		 * % gzip -l ../etc.tgz 
-		 *   compressed uncompressed  ratio uncompressed_name
-		 * 	 119737       696320  82.8% ../etc.tar
-		 */
-
-		asprintf(&cmd, "gzip -l %s", infile);
-		if ((gzipsizepipe = popen(cmd, "r")) == NULL)
-			err(1, "reading compressed file length");
-		for (; fgets(buf, 256, gzipsizepipe) != NULL;)
-		    continue;
-		strtoimax(buf, &cp, 10);
-		filesize = strtoimax(cp, NULL, 10);
-		if (pclose(gzipsizepipe) < 0)
-			err(1, "closing compressed file length pipe");
-		free(cmd);
+		filesize = gzip_uncompressed_size(infile);
 	}
 	/* Pipe input through gzip -dc if -z is given */
 	if (zflag) {
@@ -250,16 +232,18 @@ main(int argc, char *argv[])
 	cmdstat = 0;
 	while (pid || gzippid) {
 		deadpid = wait(&ws);
+		if (deadpid == -1 && errno == EINTR)
+			continue;
+		if (deadpid == -1)
+			break;
+
 		/*
 		 * We need to exit with an error if the command (or gzip)
 		 * exited abnormally.
 		 * Unfortunately we can't generate a true 'exited by signal'
 		 * error without sending the signal to ourselves :-(
 		 */
 		ws = WIFSIGNALED(ws) ? WTERMSIG(ws) : WEXITSTATUS(ws);
-
-		if (deadpid != -1 && errno == EINTR)
-			continue;
 		if (deadpid == pid) {
 			pid = 0;
 			cmdstat = ws;
@@ -280,3 +264,74 @@ main(int argc, char *argv[])
 
 	exit(cmdstat ? cmdstat : gzipstat);
 }
+
+static off_t
+gzip_uncompressed_size(const char *path)
+{
+	char buf[256];
+	char lastline[256];
+	char *cp, *endp;
+	ssize_t nr;
+	pid_t pid;
+	int status;
+	int pfd[2];
+	off_t uncompressed;
+
+	if (path == NULL)
+		errx(1, "gzip length: missing path");
+	if (pipe(pfd) < 0)
+		err(1, "pipe");
+	pid = fork();
+	if (pid < 0)
+		err(1, "fork");
+	if (pid == 0) {
+		(void)dup2(pfd[1], STDOUT_FILENO);
+		close(pfd[0]);
+		close(pfd[1]);
+		execlp("gzip", "gzip", "-l", "--", path, (char *)NULL);
+		_exit(127);
+	}
+
+	close(pfd[1]);
+	lastline[0] = '\0';
+	/*
+	 * Read all output, keep the last line.
+	 * gzip -l output contains a header; the last line has numbers.
+	 */
+	while ((nr = read(pfd[0], buf, sizeof(buf) - 1)) > 0) {
+		char *line, *nl;
+
+		buf[nr] = '\0';
+		line = buf;
+		while ((nl = strchr(line, '\n')) != NULL) {
+			size_t len = (size_t)(nl - line);
+			if (len >= sizeof(lastline))
+				len = sizeof(lastline) - 1;
+			memcpy(lastline, line, len);
+			lastline[len] = '\0';
+			line = nl + 1;
+		}
+	}
+	close(pfd[0]);
+
+	if (waitpid(pid, &status, 0) < 0)
+		err(1, "waitpid");
+	if (!WIFEXITED(status) || WEXITSTATUS(status) != 0)
+		errx(1, "gzip -l failed for %s", path);
+
+	/*
+	 * Parse: <compressed> <uncompressed> ...
+	 */
+	errno = 0;
+	(void)strtoimax(lastline, &cp, 10);
+	if (errno != 0 || cp == lastline)
+		errx(1, "unexpected gzip -l output for %s", path);
+	while (*cp == ' ' || *cp == '\t')
+		cp++;
+	errno = 0;
+	uncompressed = (off_t)strtoimax(cp, &endp, 10);
+	if (errno != 0 || endp == cp)
+		errx(1, "unexpected gzip -l output for %s", path);
+
+	return uncompressed;
+}

usr.bin/progress/tests/Makefile

@@ -0,0 +1,5 @@
+
+ATF_TESTS_SH=	progress_test
+
+.include <bsd.test.mk>
+

usr.bin/progress/tests/progress_test.sh

@@ -0,0 +1,26 @@
+#
+# Regression tests for progress(1) security/robustness.
+#
+
+atf_test_case zflag_no_shell_injection
+zflag_no_shell_injection_body()
+{
+	PROGRESS_BIN="$(ls /usr/obj/usr/src/*/usr.bin/progress/progress 2>/dev/null | head -n 1)"
+	if [ -z "${PROGRESS_BIN}" ] || [ ! -x "${PROGRESS_BIN}" ]; then
+		atf_skip "progress binary not found"
+	fi
+	atf_require_prog gzip
+
+	# Create a gzipped file whose name contains shell metacharacters.
+	echo "hello" > "in;echoINJECTED"
+	gzip -c -- "in;echoINJECTED" > "in;echoINJECTED.gz"
+
+	# If progress used popen("gzip -l %s"), this would run "echoINJECTED".
+	atf_check -s exit:0 -o not-empty -e not-match:"INJECTED" \
+	    "${PROGRESS_BIN}" -z -f "in;echoINJECTED.gz" cat >/dev/null
+}
+
+atf_init_test_cases()
+{
+	atf_add_test_case zflag_no_shell_injection
+}