Skip to content

Commit cb40f51

Browse files
committed
parse: Allow relative $PATH entries with explicit executable paths
$ bfs -execdir /bin/exe {} \; is perfectly safe regardless of what's in $PATH.
1 parent def4a83 commit cb40f51

3 files changed

Lines changed: 68 additions & 21 deletions

File tree

src/parse.c

Lines changed: 48 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1247,6 +1247,41 @@ static struct bfs_expr *parse_empty(struct bfs_parser *parser, int arg1, int arg
12471247
return expr;
12481248
}
12491249

1250+
/** Check for unsafe relative paths in $PATH. */
1251+
static const char *unsafe_path(const struct bfs_exec *execbuf) {
1252+
if (!(execbuf->flags & BFS_EXEC_CHDIR)) {
1253+
// Not -execdir or -okdir
1254+
return NULL;
1255+
}
1256+
1257+
const char *exe = execbuf->tmpl_argv[0];
1258+
if (strchr(exe, '/')) {
1259+
// No $PATH lookups for /foo or foo/bar
1260+
return NULL;
1261+
}
1262+
1263+
if (strstr(exe, "{}")) {
1264+
// Substituted paths always contain a /
1265+
return NULL;
1266+
}
1267+
1268+
const char *path = getenv("PATH");
1269+
while (path) {
1270+
if (path[0] != '/') {
1271+
// Relative $PATH component!
1272+
return path;
1273+
}
1274+
1275+
path = strchr(path, ':');
1276+
if (path) {
1277+
++path;
1278+
}
1279+
}
1280+
1281+
// No relative components in $PATH
1282+
return NULL;
1283+
}
1284+
12501285
/**
12511286
* Parse -exec(dir)?/-ok(dir)?.
12521287
*/
@@ -1269,29 +1304,21 @@ static struct bfs_expr *parse_exec(struct bfs_parser *parser, int flags, int arg
12691304
// For pipe() in bfs_spawn()
12701305
expr->ephemeral_fds = 2;
12711306

1272-
if (execbuf->flags & BFS_EXEC_CHDIR) {
1273-
// Check for relative paths in $PATH
1274-
const char *path = getenv("PATH");
1275-
while (path) {
1276-
if (*path != '/') {
1277-
size_t len = strcspn(path, ":");
1278-
char *comp = strndup(path, len);
1279-
if (comp) {
1280-
parse_expr_error(parser, expr,
1281-
"This action would be unsafe, since ${bld}$$PATH${rs} contains the relative path ${bld}%pq${rs}\n", comp);
1282-
free(comp);
1283-
} else {
1284-
parse_perror(parser, "strndup()");
1285-
}
1286-
return NULL;
1287-
}
1288-
1289-
path = strchr(path, ':');
1290-
if (path) {
1291-
++path;
1292-
}
1307+
const char *unsafe = unsafe_path(execbuf);
1308+
if (unsafe) {
1309+
size_t len = strcspn(unsafe, ":");
1310+
char *comp = strndup(unsafe, len);
1311+
if (comp) {
1312+
parse_expr_error(parser, expr,
1313+
"This action would be unsafe, since ${bld}$$PATH${rs} contains the relative path ${bld}%pq${rs}\n", comp);
1314+
free(comp);
1315+
} else {
1316+
parse_perror(parser, "strndup()");
12931317
}
1318+
return NULL;
1319+
}
12941320

1321+
if (execbuf->flags & BFS_EXEC_CHDIR) {
12951322
// To dup() the parent directory
12961323
if (execbuf->flags & BFS_EXEC_MULTI) {
12971324
++expr->persistent_fds;
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
./a
2+
./b
3+
./bar
4+
./bar
5+
./basic
6+
./baz
7+
./c
8+
./d
9+
./e
10+
./f
11+
./foo
12+
./foo
13+
./foo
14+
./g
15+
./h
16+
./i
17+
./j
18+
./k
19+
./l
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
PATH="foo:$PATH" bfs_diff basic -execdir /bin/sh -c 'printf "%s\\n" "$@"' sh {} +

0 commit comments

Comments
 (0)