From f1d40a3c7c8891eadd0cf59419adb8cc70fd2768 Mon Sep 17 00:00:00 2001 From: Kurtis Rader Date: Sat, 22 Apr 2017 20:33:56 -0700 Subject: [PATCH] limit `bind` mode names to the rules for var names The bind mode names can be, and are, used in the construction of fish variable names. So don't allow users to use names that are not legal as a variable name. This should not break anything since, AFAICT, no existing fish scripts, including those provided by Oh-My-Fish and Fisherman define bind modes that would not be legal with this change. Fixes #3965 --- src/builtin.cpp | 85 ++++++++++++++++++++++++------------------------- src/builtin.h | 3 ++ tests/bind.err | 4 +++ tests/bind.in | 11 +++++++ tests/bind.out | 0 5 files changed, 59 insertions(+), 44 deletions(-) create mode 100644 tests/bind.err create mode 100644 tests/bind.in create mode 100644 tests/bind.out diff --git a/src/builtin.cpp b/src/builtin.cpp index 8e69b4ea2..f26015fe7 100644 --- a/src/builtin.cpp +++ b/src/builtin.cpp @@ -450,86 +450,84 @@ static void builtin_bind_list_modes(io_streams_t &streams) { /// The bind builtin, used for setting character sequences. static int builtin_bind(parser_t &parser, io_streams_t &streams, wchar_t **argv) { - wgetopter_t w; enum { BIND_INSERT, BIND_ERASE, BIND_KEY_NAMES, BIND_FUNCTION_NAMES }; + wchar_t *cmd = argv[0]; int argc = builtin_count_args(argv); int mode = BIND_INSERT; int res = STATUS_BUILTIN_OK; - int all = 0; + bool all = false; + bool use_terminfo = false; const wchar_t *bind_mode = DEFAULT_BIND_MODE; bool bind_mode_given = false; const wchar_t *sets_bind_mode = L""; - int use_terminfo = 0; - w.woptind = 0; - - static const struct woption long_options[] = {{L"all", no_argument, 0, 'a'}, - {L"erase", no_argument, 0, 'e'}, - {L"function-names", no_argument, 0, 'f'}, - {L"help", no_argument, 0, 'h'}, - {L"key", no_argument, 0, 'k'}, - {L"key-names", no_argument, 0, 'K'}, - {L"mode", required_argument, 0, 'M'}, - {L"list-modes", no_argument, 0, 'L'}, - {L"sets-mode", required_argument, 0, 'm'}, - {0, 0, 0, 0}}; - - while (1) { - int opt_index = 0; - int opt = w.wgetopt_long_only(argc, argv, L"aehkKfM:Lm:", long_options, &opt_index); - if (opt == -1) break; + static const wchar_t *short_options = L"aehkKfM:Lm:"; + static const struct woption long_options[] = {{L"all", no_argument, NULL, 'a'}, + {L"erase", no_argument, NULL, 'e'}, + {L"function-names", no_argument, NULL, 'f'}, + {L"help", no_argument, NULL, 'h'}, + {L"key", no_argument, NULL, 'k'}, + {L"key-names", no_argument, NULL, 'K'}, + {L"mode", required_argument, NULL, 'M'}, + {L"list-modes", no_argument, NULL, 'L'}, + {L"sets-mode", required_argument, NULL, 'm'}, + {NULL, 0, NULL, 0}}; + int opt; + wgetopter_t w; + while ((opt = w.wgetopt_long_only(argc, argv, short_options, long_options, NULL)) != -1) { switch (opt) { - case 0: { - if (long_options[opt_index].flag != 0) break; - streams.err.append_format(BUILTIN_ERR_UNKNOWN, argv[0], - long_options[opt_index].name); - builtin_print_help(parser, streams, argv[0], streams.err); - return STATUS_BUILTIN_ERROR; - } - case 'a': { - all = 1; + case L'a': { + all = true; break; } - case 'e': { + case L'e': { mode = BIND_ERASE; break; } - case 'h': { + case L'h': { builtin_print_help(parser, streams, argv[0], streams.out); return STATUS_BUILTIN_OK; } - case 'k': { - use_terminfo = 1; + case L'k': { + use_terminfo = true; break; } - case 'K': { + case L'K': { mode = BIND_KEY_NAMES; break; } - case 'f': { + case L'f': { mode = BIND_FUNCTION_NAMES; break; } - case 'M': { + case L'M': { + if (!valid_var_name(w.woptarg)) { + streams.err.append_format(BUILTIN_ERR_BIND_MODE, cmd, w.woptarg); + return STATUS_BUILTIN_ERROR; + } bind_mode = w.woptarg; bind_mode_given = true; break; } - case 'm': { + case L'm': { + if (!valid_var_name(w.woptarg)) { + streams.err.append_format(BUILTIN_ERR_BIND_MODE, cmd, w.woptarg); + return STATUS_BUILTIN_ERROR; + } sets_bind_mode = w.woptarg; break; } - case 'L': { + case L'L': { builtin_bind_list_modes(streams); return STATUS_BUILTIN_OK; } - case '?': { + case L'?': { builtin_unknown_option(parser, streams, argv[0], argv[w.woptind - 1]); return STATUS_BUILTIN_ERROR; } default: { - DIE("unexpected opt"); + DIE("unexpected retval from wgetopt_long_only"); break; } } @@ -1668,7 +1666,7 @@ int builtin_function(parser_t &parser, io_streams_t &streams, const wcstring_lis } case 'v': { if (!valid_var_name(w.woptarg)) { - append_format(*out_err, _(L"%ls: Invalid variable name '%ls'"), cmd, w.woptarg); + append_format(*out_err, BUILTIN_ERR_VARNAME, cmd, w.woptarg); return STATUS_BUILTIN_ERROR; } @@ -1738,10 +1736,9 @@ int builtin_function(parser_t &parser, io_streams_t &streams, const wcstring_lis } case 'V': { if (!valid_var_name(w.woptarg)) { - append_format(*out_err, _(L"%ls: Invalid variable name '%ls'"), cmd, w.woptarg); + append_format(*out_err, BUILTIN_ERR_VARNAME, cmd, w.woptarg); return STATUS_BUILTIN_ERROR; } - inherit_vars.push_back(w.woptarg); break; } @@ -2110,10 +2107,10 @@ static int read_one_char_at_a_time(int fd, wcstring &buff, int nchars, bool spli /// The read builtin. Reads from stdin and stores the values in environment variables. static int builtin_read(parser_t &parser, io_streams_t &streams, wchar_t **argv) { - wcstring buff; wchar_t *cmd = argv[0]; int argc = builtin_count_args(argv); int place = ENV_USER; + wcstring buff; wcstring prompt_cmd; const wchar_t *prompt = NULL; const wchar_t *prompt_str = NULL; diff --git a/src/builtin.h b/src/builtin.h index 239abc008..4e2f0612e 100644 --- a/src/builtin.h +++ b/src/builtin.h @@ -60,6 +60,9 @@ enum { COMMAND_NOT_BUILTIN, BUILTIN_REGULAR, BUILTIN_FUNCTION }; /// Error message for invalid variable name. #define BUILTIN_ERR_VARNAME _(L"%ls: Variable name '%ls' is not valid. See `help identifiers`.\n") +/// Error message for invalid bind mode name. +#define BUILTIN_ERR_BIND_MODE _(L"%ls: mode name '%ls' is not valid. See `help identifiers`.\n") + /// Error message when too many arguments are supplied to a builtin. #define BUILTIN_ERR_TOO_MANY_ARGUMENTS _(L"%ls: Too many arguments\n") diff --git a/tests/bind.err b/tests/bind.err new file mode 100644 index 000000000..68fd269ef --- /dev/null +++ b/tests/bind.err @@ -0,0 +1,4 @@ +# Verify that an invalid bind mode is rejected. +bind: mode name 'bad bind mode' is not valid. See `help identifiers`. +# Verify that an invalid bind mode target is rejected. +bind: mode name 'bind-mode' is not valid. See `help identifiers`. diff --git a/tests/bind.in b/tests/bind.in new file mode 100644 index 000000000..7fbf373ad --- /dev/null +++ b/tests/bind.in @@ -0,0 +1,11 @@ +# Test various `bind` command invocations. This is meant to verify that +# invalid flags, mode names, etc. are caught as well as to verify that valid +# ones are allowed. + +echo \# Verify that an invalid bind mode is rejected. >&2 +bind -m 'bad bind mode' \cX true +echo \# Verify that an invalid bind mode target is rejected. >&2 +bind -M bind-mode \cX true + +# This should succeed and result in a success, zero, status. +bind -M bind_mode \cX true diff --git a/tests/bind.out b/tests/bind.out new file mode 100644 index 000000000..e69de29bb