From b5ebe959cca84101795ec29022af5fcbcb94ed78 Mon Sep 17 00:00:00 2001
From: Tavian Barnes <tavianator@tavianator.com>
Date: Sat, 29 Jul 2017 19:10:03 -0400
Subject: exec: Don't allow anything between {} and +

POSIX explicitly forbids this extension:

> Only a <plus-sign> that immediately follows an argument containing
> only the two characters "{}" shall punctuate the end of the primary
> expression. Other uses of the <plus-sign> shall not be treated as
> special.
---
 exec.c                                | 105 ++++++++++------------------------
 exec.h                                |   3 -
 tests.sh                              |  58 ++++++++++---------
 tests/test_exec_plus_semicolon.out    |  19 ++++++
 tests/test_exec_plus_substring.out    |   1 -
 tests/test_execdir_plus_semicolon.out |  19 ++++++
 tests/test_execdir_plus_substring.out |  11 ----
 7 files changed, 99 insertions(+), 117 deletions(-)
 create mode 100644 tests/test_exec_plus_semicolon.out
 delete mode 100644 tests/test_exec_plus_substring.out
 create mode 100644 tests/test_execdir_plus_semicolon.out
 delete mode 100644 tests/test_execdir_plus_substring.out

diff --git a/exec.c b/exec.c
index 1884536..250bba7 100644
--- a/exec.c
+++ b/exec.c
@@ -79,11 +79,8 @@ static size_t bfs_exec_arg_max(const struct bfs_exec *execbuf) {
 	}
 	bfs_exec_debug(execbuf, "ARG_MAX: %ld remaining after environment variables\n", arg_max);
 
-	// Account for the non-placeholder arguments
-	for (size_t i = 0; i < execbuf->placeholder; ++i) {
-		arg_max -= bfs_exec_arg_size(execbuf->tmpl_argv[i]);
-	}
-	for (size_t i = execbuf->placeholder + 1; i < execbuf->tmpl_argc; ++i) {
+	// Account for the fixed arguments
+	for (size_t i = 0; i < execbuf->tmpl_argc - 1; ++i) {
 		arg_max -= bfs_exec_arg_size(execbuf->tmpl_argv[i]);
 	}
 	bfs_exec_debug(execbuf, "ARG_MAX: %ld remaining after fixed arguments\n", arg_max);
@@ -112,7 +109,6 @@ struct bfs_exec *parse_bfs_exec(char **argv, enum bfs_exec_flags flags, const st
 	}
 
 	execbuf->flags = flags;
-	execbuf->placeholder = 0;
 	execbuf->argv = NULL;
 	execbuf->argc = 0;
 	execbuf->argv_cap = 0;
@@ -128,19 +124,18 @@ struct bfs_exec *parse_bfs_exec(char **argv, enum bfs_exec_flags flags, const st
 	}
 
 	size_t i;
-	const char *arg;
-	for (i = 1, arg = argv[i]; arg; arg = argv[++i]) {
-		if (strcmp(arg, ";") == 0) {
+	for (i = 1; ; ++i) {
+		const char *arg = argv[i];
+		if (!arg) {
+			cfprintf(cerr, "%{er}error: %s: Expected ';' or '+'.%{rs}\n", argv[0]);
+			goto fail;
+		} else if (strcmp(arg, ";") == 0) {
 			break;
-		} else if (strcmp(arg, "+") == 0) {
+		} else if (strcmp(arg, "+") == 0 && strcmp(argv[i - 1], "{}") == 0) {
 			execbuf->flags |= BFS_EXEC_MULTI;
 			break;
 		}
 	}
-	if (!arg) {
-		cfprintf(cerr, "%{er}error: %s: Expected ';' or '+'.%{rs}\n", argv[0]);
-		goto fail;
-	}
 
 	if ((execbuf->flags & BFS_EXEC_CONFIRM) && (execbuf->flags & BFS_EXEC_MULTI)) {
 		cfprintf(cerr, "%{er}error: %s ... + is not supported.%{rs}\n", argv[0]);
@@ -158,29 +153,17 @@ struct bfs_exec *parse_bfs_exec(char **argv, enum bfs_exec_flags flags, const st
 	}
 
 	if (execbuf->flags & BFS_EXEC_MULTI) {
-		for (i = 0; i < execbuf->tmpl_argc; ++i) {
-			if (strstr(execbuf->tmpl_argv[i], "{}")) {
-				execbuf->placeholder = i;
-				break;
-			}
-		}
-		if (i == execbuf->tmpl_argc) {
-			cfprintf(cerr, "%{er}error: %s ... +: Expected '{}'.%{rs}\n", argv[0]);
-			goto fail;
-		}
-		for (++i; i < execbuf->tmpl_argc; ++i) {
-			if (strstr(execbuf->tmpl_argv[i], "{}")) {
+		for (i = 0; i < execbuf->tmpl_argc - 1; ++i) {
+			char *arg = execbuf->tmpl_argv[i];
+			if (strstr(arg, "{}")) {
 				cfprintf(cerr, "%{er}error: %s ... +: Only one '{}' is supported.%{rs}\n", argv[0]);
 				goto fail;
 			}
+			execbuf->argv[i] = arg;
 		}
+		execbuf->argc = execbuf->tmpl_argc - 1;
 
 		execbuf->arg_max = bfs_exec_arg_max(execbuf);
-
-		for (i = 0; i < execbuf->placeholder; ++i) {
-			execbuf->argv[i] = execbuf->tmpl_argv[i];
-		}
-		execbuf->argc = i;
 	}
 
 	return execbuf;
@@ -191,43 +174,28 @@ fail:
 }
 
 /** Format the current path for use as a command line argument. */
-static const char *bfs_exec_format_path(const struct bfs_exec *execbuf, const struct BFTW *ftwbuf) {
+static char *bfs_exec_format_path(const struct bfs_exec *execbuf, const struct BFTW *ftwbuf) {
 	if (!(execbuf->flags & BFS_EXEC_CHDIR)) {
-		return ftwbuf->path;
+		return strdup(ftwbuf->path);
 	}
 
 	const char *name = ftwbuf->path + ftwbuf->nameoff;
 
 	if (name[0] == '/') {
 		// Must be a root path ("/", "//", etc.)
-		return name;
+		return strdup(name);
 	}
 
 	// For compatibility with GNU find, use './name' instead of just 'name'
-	char *path = dstralloc(2 + strlen(name));
+	char *path = malloc(2 + strlen(name) + 1);
 	if (!path) {
 		return NULL;
 	}
 
-	if (dstrcat(&path, "./") != 0) {
-		goto err;
-	}
-	if (dstrcat(&path, name) != 0) {
-		goto err;
-	}
+	strcpy(path, "./");
+	strcpy(path + 2, name);
 
 	return path;
-
-err:
-	dstrfree(path);
-	return NULL;
-}
-
-/** Free a formatted path. */
-static void bfs_exec_free_path(const char *path, const struct BFTW *ftwbuf) {
-	if (path != ftwbuf->path && path != ftwbuf->path + ftwbuf->nameoff) {
-		dstrfree((char *)path);
-	}
 }
 
 /** Format an argument, expanding "{}" to the current path. */
@@ -388,7 +356,7 @@ fail:
 static int bfs_exec_single(struct bfs_exec *execbuf, const struct BFTW *ftwbuf) {
 	int ret = -1, error = 0;
 
-	const char *path = bfs_exec_format_path(execbuf, ftwbuf);
+	char *path = bfs_exec_format_path(execbuf, ftwbuf);
 	if (!path) {
 		goto out;
 	}
@@ -420,7 +388,7 @@ out_free:
 		bfs_exec_free_arg(execbuf->argv[j], execbuf->tmpl_argv[j]);
 	}
 
-	bfs_exec_free_path(path, ftwbuf);
+	free(path);
 
 	errno = error;
 
@@ -432,13 +400,8 @@ out:
 static int bfs_exec_flush(struct bfs_exec *execbuf) {
 	int ret = 0;
 
-	size_t last_path = execbuf->argc;
-	if (last_path > execbuf->placeholder) {
-		for (size_t i = execbuf->placeholder + 1; i < execbuf->tmpl_argc; ++i, ++execbuf->argc) {
-			execbuf->argv[execbuf->argc] = execbuf->tmpl_argv[i];
-		}
+	if (execbuf->argc > execbuf->tmpl_argc - 1) {
 		execbuf->argv[execbuf->argc] = NULL;
-
 		if (bfs_exec_spawn(execbuf) != 0) {
 			ret = -1;
 		}
@@ -448,10 +411,10 @@ static int bfs_exec_flush(struct bfs_exec *execbuf) {
 
 	bfs_exec_closewd(execbuf, NULL);
 
-	for (size_t i = execbuf->placeholder; i < last_path; ++i) {
-		bfs_exec_free_arg(execbuf->argv[i], execbuf->tmpl_argv[execbuf->placeholder]);
+	for (size_t i = execbuf->tmpl_argc - 1; i < execbuf->argc; ++i) {
+		free(execbuf->argv[i]);
 	}
-	execbuf->argc = execbuf->placeholder;
+	execbuf->argc = execbuf->tmpl_argc - 1;
 	execbuf->arg_size = 0;
 
 	errno = error;
@@ -499,16 +462,10 @@ static int bfs_exec_push(struct bfs_exec *execbuf, char *arg) {
 static int bfs_exec_multi(struct bfs_exec *execbuf, const struct BFTW *ftwbuf) {
 	int ret = 0;
 
-	const char *path = bfs_exec_format_path(execbuf, ftwbuf);
-	if (!path) {
-		ret = -1;
-		goto out;
-	}
-
-	char *arg = bfs_exec_format_arg(execbuf->tmpl_argv[execbuf->placeholder], path);
+	char *arg = bfs_exec_format_path(execbuf, ftwbuf);
 	if (!arg) {
 		ret = -1;
-		goto out_path;
+		goto out;
 	}
 
 	if (bfs_exec_needs_flush(execbuf, ftwbuf, arg)) {
@@ -530,12 +487,10 @@ static int bfs_exec_multi(struct bfs_exec *execbuf, const struct BFTW *ftwbuf) {
 	}
 
 	// arg will get cleaned up later by bfs_exec_flush()
-	goto out_path;
+	goto out;
 
 out_arg:
-	bfs_exec_free_arg(arg, execbuf->tmpl_argv[execbuf->placeholder]);
-out_path:
-	bfs_exec_free_path(path, ftwbuf);
+	free(arg);
 out:
 	return ret;
 }
diff --git a/exec.h b/exec.h
index 751a963..1a75064 100644
--- a/exec.h
+++ b/exec.h
@@ -48,9 +48,6 @@ struct bfs_exec {
 	/** Command line template size. */
 	size_t tmpl_argc;
 
-	/** For BFS_EXEC_MULTI, the index of the placeholder argument. */
-	size_t placeholder;
-
 	/** The built command line. */
 	char **argv;
 	/** Number of command line arguments. */
diff --git a/tests.sh b/tests.sh
index 53d7189..dcecb01 100755
--- a/tests.sh
+++ b/tests.sh
@@ -187,6 +187,7 @@ posix_tests=(
     test_size_bytes
     test_exec
     test_exec_plus
+    test_exec_plus_semicolon
     test_flag_comma
     test_perm_222
     test_perm_222_minus
@@ -224,15 +225,15 @@ bsd_tests=(
     test_size_big
     test_exec_substring
     test_execdir_pwd
+    test_execdir_slash
+    test_execdir_slash_pwd
+    test_execdir_slashes
     test_double_dash
     test_flag_double_dash
     test_ok_stdin
     test_okdir_stdin
     test_delete
     test_rm
-    test_execdir_slash
-    test_execdir_slash_pwd
-    test_execdir_slashes
     test_regex
     test_iregex
     test_regex_parens
@@ -301,7 +302,11 @@ gnu_tests=(
     test_exec_substring
     test_execdir
     test_execdir_substring
+    test_execdir_plus_semicolon
     test_execdir_pwd
+    test_execdir_slash
+    test_execdir_slash_pwd
+    test_execdir_slashes
     test_weird_names
     test_flag_weird_names
     test_follow_comma
@@ -316,9 +321,6 @@ gnu_tests=(
     test_perm_symbolic_slash
     test_perm_leading_plus_symbolic_slash
     test_delete
-    test_execdir_slash
-    test_execdir_slash_pwd
-    test_execdir_slashes
     test_regex
     test_iregex
     test_regex_parens
@@ -363,9 +365,7 @@ bfs_tests=(
     test_perm_symbolic_missing_action
     test_perm_leading_plus_symbolic
     test_perm_octal_plus
-    test_exec_plus_substring
     test_execdir_plus
-    test_execdir_plus_substring
     test_hidden
     test_nohidden
     test_path_flag_expr
@@ -746,12 +746,16 @@ function test_exec_plus() {
     bfs_diff basic -exec "$TESTS/sort-args.sh" '{}' +
 }
 
-function test_exec_substring() {
-    bfs_diff basic -exec echo '-{}-' ';'
+function test_exec_plus_semicolon() {
+    # POSIX says:
+    #     Only a <plus-sign> that immediately follows an argument containing only the two characters "{}"
+    #     shall punctuate the end of the primary expression. Other uses of the <plus-sign> shall not be
+    #     treated as special.
+    bfs_diff basic -exec "$TESTS/sort-args.sh" foo '{}' bar + baz \;
 }
 
-function test_exec_plus_substring() {
-    bfs_diff basic -exec "$TESTS/sort-args.sh" a '-{}-' z +
+function test_exec_substring() {
+    bfs_diff basic -exec echo '-{}-' ';'
 }
 
 function test_execdir() {
@@ -766,8 +770,8 @@ function test_execdir_substring() {
     bfs_diff basic -execdir echo '-{}-' ';'
 }
 
-function test_execdir_plus_substring() {
-    bfs_diff basic -execdir "$TESTS/sort-args.sh" a '-{}-' z +
+function test_execdir_plus_semicolon() {
+    bfs_diff basic -execdir "$TESTS/sort-args.sh" foo '{}' bar + baz \;
 }
 
 function test_execdir_pwd() {
@@ -776,6 +780,19 @@ function test_execdir_pwd() {
     bfs_diff basic -execdir bash -c "pwd | cut -b$OFFSET-" ';'
 }
 
+function test_execdir_slash() {
+    # Don't prepend ./ for absolute paths in -execdir
+    bfs_diff / -maxdepth 0 -execdir echo '{}' ';'
+}
+
+function test_execdir_slash_pwd() {
+    bfs_diff / -maxdepth 0 -execdir pwd ';'
+}
+
+function test_execdir_slashes() {
+    bfs_diff /// -maxdepth 0 -execdir echo '{}' ';'
+}
+
 function test_weird_names() {
     cd weirdnames
     bfs_diff '-' '(-' '!-' ',' ')' './(' './!' \( \! -print , -print \)
@@ -938,19 +955,6 @@ function test_rm() {
     bfs_diff scratch
 }
 
-function test_execdir_slash() {
-    # Don't prepend ./ for absolute paths in -execdir
-    bfs_diff / -maxdepth 0 -execdir echo '{}' ';'
-}
-
-function test_execdir_slash_pwd() {
-    bfs_diff / -maxdepth 0 -execdir pwd ';'
-}
-
-function test_execdir_slashes() {
-    bfs_diff /// -maxdepth 0 -execdir echo '{}' ';'
-}
-
 function test_regex() {
     bfs_diff basic -regex 'basic/./.'
 }
diff --git a/tests/test_exec_plus_semicolon.out b/tests/test_exec_plus_semicolon.out
new file mode 100644
index 0000000..27887e2
--- /dev/null
+++ b/tests/test_exec_plus_semicolon.out
@@ -0,0 +1,19 @@
++ bar basic baz foo
++ bar basic/a baz foo
++ bar basic/b baz foo
++ bar basic/c baz foo
++ bar basic/e baz foo
++ bar basic/g baz foo
++ bar basic/i baz foo
++ bar basic/j baz foo
++ bar basic/k baz foo
++ bar basic/l baz foo
++ bar basic/c/d baz foo
++ bar basic/e/f baz foo
++ bar basic/g/h baz foo
++ bar basic/j/foo baz foo
++ bar basic/k/foo baz foo
++ bar basic/l/foo baz foo
++ bar basic/k/foo/bar baz foo
++ bar basic/l/foo/bar baz foo
++ bar basic/l/foo/bar/baz baz foo
diff --git a/tests/test_exec_plus_substring.out b/tests/test_exec_plus_substring.out
deleted file mode 100644
index 7e5bb34..0000000
--- a/tests/test_exec_plus_substring.out
+++ /dev/null
@@ -1 +0,0 @@
--basic- -basic/a- -basic/b- -basic/c- -basic/c/d- -basic/e- -basic/e/f- -basic/g- -basic/g/h- -basic/i- -basic/j- -basic/j/foo- -basic/k- -basic/k/foo- -basic/k/foo/bar- -basic/l- -basic/l/foo- -basic/l/foo/bar- -basic/l/foo/bar/baz- a z
diff --git a/tests/test_execdir_plus_semicolon.out b/tests/test_execdir_plus_semicolon.out
new file mode 100644
index 0000000..9acfc5d
--- /dev/null
+++ b/tests/test_execdir_plus_semicolon.out
@@ -0,0 +1,19 @@
++ ./a bar baz foo
++ ./b bar baz foo
++ ./bar bar baz foo
++ ./bar bar baz foo
++ ./basic bar baz foo
++ ./baz bar baz foo
++ ./c bar baz foo
++ ./d bar baz foo
++ ./e bar baz foo
++ ./f bar baz foo
++ ./foo bar baz foo
++ ./foo bar baz foo
++ ./foo bar baz foo
++ ./g bar baz foo
++ ./h bar baz foo
++ ./i bar baz foo
++ ./j bar baz foo
++ ./k bar baz foo
++ ./l bar baz foo
diff --git a/tests/test_execdir_plus_substring.out b/tests/test_execdir_plus_substring.out
deleted file mode 100644
index 838fbae..0000000
--- a/tests/test_execdir_plus_substring.out
+++ /dev/null
@@ -1,11 +0,0 @@
--./bar- a z
--./bar- a z
--./basic- a z
--./baz- a z
--./d- a z
--./f- a z
--./foo- a z
--./foo- a z
--./foo- a z
--./h- a z
--./a- -./b- -./c- -./e- -./g- -./i- -./j- -./k- -./l- a z
-- 
cgit v1.2.3