Skip to content

Commit

Permalink
exec: Don't allow anything between {} and +
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
tavianator committed Jul 29, 2017
1 parent 5361266 commit b5ebe95
Show file tree
Hide file tree
Showing 7 changed files with 99 additions and 117 deletions.
105 changes: 30 additions & 75 deletions exec.c
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand All @@ -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]);
Expand All @@ -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;
Expand All @@ -191,43 +174,28 @@ struct bfs_exec *parse_bfs_exec(char **argv, enum bfs_exec_flags flags, const st
}

/** 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. */
Expand Down Expand Up @@ -388,7 +356,7 @@ static int bfs_exec_spawn(const struct bfs_exec *execbuf) {
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;
}
Expand Down Expand Up @@ -420,7 +388,7 @@ static int bfs_exec_single(struct bfs_exec *execbuf, const struct BFTW *ftwbuf)
bfs_exec_free_arg(execbuf->argv[j], execbuf->tmpl_argv[j]);
}

bfs_exec_free_path(path, ftwbuf);
free(path);

errno = error;

Expand All @@ -432,13 +400,8 @@ static int bfs_exec_single(struct bfs_exec *execbuf, const struct BFTW *ftwbuf)
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;
}
Expand All @@ -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;
Expand Down Expand Up @@ -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)) {
Expand All @@ -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;
}
Expand Down
3 changes: 0 additions & 3 deletions exec.h
Expand Up @@ -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. */
Expand Down
58 changes: 31 additions & 27 deletions tests.sh
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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() {
Expand All @@ -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() {
Expand All @@ -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 \)
Expand Down Expand Up @@ -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/./.'
}
Expand Down
19 changes: 19 additions & 0 deletions 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
1 change: 0 additions & 1 deletion tests/test_exec_plus_substring.out

This file was deleted.

19 changes: 19 additions & 0 deletions 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

0 comments on commit b5ebe95

Please sign in to comment.