From cb40f51e4e6375a10265484b6959c6b1b0591378 Mon Sep 17 00:00:00 2001
From: Tavian Barnes <tavianator@tavianator.com>
Date: Sun, 15 Jun 2025 09:57:45 -0400
Subject: parse: Allow relative $PATH entries with explicit executable paths

    $ bfs -execdir /bin/exe {} \;

is perfectly safe regardless of what's in $PATH.
---
 src/parse.c                               | 69 +++++++++++++++++++++----------
 tests/bfs/execdir_path_relative_slash.out | 19 +++++++++
 tests/bfs/execdir_path_relative_slash.sh  |  1 +
 3 files changed, 68 insertions(+), 21 deletions(-)
 create mode 100644 tests/bfs/execdir_path_relative_slash.out
 create mode 100644 tests/bfs/execdir_path_relative_slash.sh

diff --git a/src/parse.c b/src/parse.c
index 9c39d6b..5ec4c0e 100644
--- a/src/parse.c
+++ b/src/parse.c
@@ -1247,6 +1247,41 @@ static struct bfs_expr *parse_empty(struct bfs_parser *parser, int arg1, int arg
 	return expr;
 }
 
+/** Check for unsafe relative paths in $PATH. */
+static const char *unsafe_path(const struct bfs_exec *execbuf) {
+	if (!(execbuf->flags & BFS_EXEC_CHDIR)) {
+		// Not -execdir or -okdir
+		return NULL;
+	}
+
+	const char *exe = execbuf->tmpl_argv[0];
+	if (strchr(exe, '/')) {
+		// No $PATH lookups for /foo or foo/bar
+		return NULL;
+	}
+
+	if (strstr(exe, "{}")) {
+		// Substituted paths always contain a /
+		return NULL;
+	}
+
+	const char *path = getenv("PATH");
+	while (path) {
+		if (path[0] != '/') {
+			// Relative $PATH component!
+			return path;
+		}
+
+		path = strchr(path, ':');
+		if (path) {
+			++path;
+		}
+	}
+
+	// No relative components in $PATH
+	return NULL;
+}
+
 /**
  * Parse -exec(dir)?/-ok(dir)?.
  */
@@ -1269,29 +1304,21 @@ static struct bfs_expr *parse_exec(struct bfs_parser *parser, int flags, int arg
 	// For pipe() in bfs_spawn()
 	expr->ephemeral_fds = 2;
 
-	if (execbuf->flags & BFS_EXEC_CHDIR) {
-		// Check for relative paths in $PATH
-		const char *path = getenv("PATH");
-		while (path) {
-			if (*path != '/') {
-				size_t len = strcspn(path, ":");
-				char *comp = strndup(path, len);
-				if (comp) {
-					parse_expr_error(parser, expr,
-						"This action would be unsafe, since ${bld}$$PATH${rs} contains the relative path ${bld}%pq${rs}\n", comp);
-					free(comp);
-				} else {
-					parse_perror(parser, "strndup()");
-				}
-				return NULL;
-			}
-
-			path = strchr(path, ':');
-			if (path) {
-				++path;
-			}
+	const char *unsafe = unsafe_path(execbuf);
+	if (unsafe) {
+		size_t len = strcspn(unsafe, ":");
+		char *comp = strndup(unsafe, len);
+		if (comp) {
+			parse_expr_error(parser, expr,
+				"This action would be unsafe, since ${bld}$$PATH${rs} contains the relative path ${bld}%pq${rs}\n", comp);
+			free(comp);
+		} else {
+			parse_perror(parser, "strndup()");
 		}
+		return NULL;
+	}
 
+	if (execbuf->flags & BFS_EXEC_CHDIR) {
 		// To dup() the parent directory
 		if (execbuf->flags & BFS_EXEC_MULTI) {
 			++expr->persistent_fds;
diff --git a/tests/bfs/execdir_path_relative_slash.out b/tests/bfs/execdir_path_relative_slash.out
new file mode 100644
index 0000000..62b31f6
--- /dev/null
+++ b/tests/bfs/execdir_path_relative_slash.out
@@ -0,0 +1,19 @@
+./a
+./b
+./bar
+./bar
+./basic
+./baz
+./c
+./d
+./e
+./f
+./foo
+./foo
+./foo
+./g
+./h
+./i
+./j
+./k
+./l
diff --git a/tests/bfs/execdir_path_relative_slash.sh b/tests/bfs/execdir_path_relative_slash.sh
new file mode 100644
index 0000000..fb5a924
--- /dev/null
+++ b/tests/bfs/execdir_path_relative_slash.sh
@@ -0,0 +1 @@
+PATH="foo:$PATH" bfs_diff basic -execdir /bin/sh -c 'printf "%s\\n" "$@"' sh {} +
-- 
cgit v1.2.3