stty: ensure no side effects from invalid options

* src/stty.c (apply_settings): A new function refactored
from main() that is used to both check and apply options.
(main): Call apply_settings before we open the device,
so all validation is done before interacting with a device.
* NEWS: Mention the improvement.
* tests/misc/stty.sh: Add a test case.
This commit is contained in:
Pádraig Brady 2017-01-09 00:07:42 +00:00
parent 229431d63c
commit 9c0a3a27f7
3 changed files with 223 additions and 176 deletions

5
NEWS
View File

@ -13,6 +13,11 @@ GNU coreutils NEWS -*- outline -*-
depending on the size of the first file processed.
[bug introduced in coreutils-8.24]
** Improvements
stty now validates arguments before interacting with the device,
ensuring there are no side effects to specifying an invalid option.
* Noteworthy changes in release 8.26 (2016-11-30) [stable]

View File

@ -1077,6 +1077,206 @@ settings, CHAR is taken literally, or coded as in ^c, 0x37, 0177 or\n\
exit (status);
}
/* Apply specified settings to MODE, and update
SPEED_WAS_SET and REQUIRE_SET_ATTR as required.
If CHECKING is true, this function doesn't interact
with a device, and only validates specified settings. */
static void
apply_settings (bool checking, const char *device_name,
char * const *settings, int n_settings,
struct termios *mode, bool *speed_was_set,
bool *require_set_attr)
{
for (int k = 1; k < n_settings; k++)
{
char const *arg = settings[k];
bool match_found = false;
bool not_set_attr = false;
bool reversed = false;
int i;
if (! arg)
continue;
if (arg[0] == '-')
{
++arg;
reversed = true;
}
if (STREQ (arg, "drain"))
{
tcsetattr_options = reversed ? TCSANOW : TCSADRAIN;
continue;
}
for (i = 0; mode_info[i].name != NULL; ++i)
{
if (STREQ (arg, mode_info[i].name))
{
if ((mode_info[i].flags & NO_SETATTR) == 0)
{
match_found = set_mode (&mode_info[i], reversed, mode);
*require_set_attr = true;
}
else
match_found = not_set_attr = true;
break;
}
}
if (!match_found && reversed)
{
error (0, 0, _("invalid argument %s"), quote (arg - 1));
usage (EXIT_FAILURE);
}
if (!match_found)
{
for (i = 0; control_info[i].name != NULL; ++i)
{
if (STREQ (arg, control_info[i].name))
{
if (k == n_settings - 1)
{
error (0, 0, _("missing argument to %s"), quote (arg));
usage (EXIT_FAILURE);
}
match_found = true;
++k;
set_control_char (&control_info[i], settings[k], mode);
*require_set_attr = true;
break;
}
}
}
if (!match_found || not_set_attr)
{
if (STREQ (arg, "ispeed"))
{
if (k == n_settings - 1)
{
error (0, 0, _("missing argument to %s"), quote (arg));
usage (EXIT_FAILURE);
}
++k;
if (checking)
continue;
set_speed (input_speed, settings[k], mode);
*speed_was_set = true;
*require_set_attr = true;
}
else if (STREQ (arg, "ospeed"))
{
if (k == n_settings - 1)
{
error (0, 0, _("missing argument to %s"), quote (arg));
usage (EXIT_FAILURE);
}
++k;
if (checking)
continue;
set_speed (output_speed, settings[k], mode);
*speed_was_set = true;
*require_set_attr = true;
}
#ifdef TIOCEXT
/* This is the BSD interface to "extproc".
Even though it's an lflag, an ioctl is used to set it. */
else if (STREQ (arg, "extproc"))
{
int val = ! reversed;
if (checking)
continue;
if (ioctl (STDIN_FILENO, TIOCEXT, &val) != 0)
{
die (EXIT_FAILURE, errno, _("%s: error setting %s"),
quotef_n (0, device_name), quote_n (1, arg));
}
}
#endif
#ifdef TIOCGWINSZ
else if (STREQ (arg, "rows"))
{
if (k == n_settings - 1)
{
error (0, 0, _("missing argument to %s"), quote (arg));
usage (EXIT_FAILURE);
}
++k;
if (checking)
continue;
set_window_size (integer_arg (settings[k], INT_MAX), -1,
device_name);
}
else if (STREQ (arg, "cols")
|| STREQ (arg, "columns"))
{
if (k == n_settings - 1)
{
error (0, 0, _("missing argument to %s"), quote (arg));
usage (EXIT_FAILURE);
}
++k;
if (checking)
continue;
set_window_size (-1, integer_arg (settings[k], INT_MAX),
device_name);
}
else if (STREQ (arg, "size"))
{
if (checking)
continue;
max_col = screen_columns ();
current_col = 0;
display_window_size (false, device_name);
}
#endif
#ifdef HAVE_C_LINE
else if (STREQ (arg, "line"))
{
unsigned long int value;
if (k == n_settings - 1)
{
error (0, 0, _("missing argument to %s"), quote (arg));
usage (EXIT_FAILURE);
}
++k;
mode->c_line = value = integer_arg (settings[k], ULONG_MAX);
if (mode->c_line != value)
error (0, 0, _("invalid line discipline %s"),
quote (settings[k]));
*require_set_attr = true;
}
#endif
else if (STREQ (arg, "speed"))
{
if (checking)
continue;
max_col = screen_columns ();
display_speed (mode, false);
}
else if (string_to_baud (arg) != (speed_t) -1)
{
if (checking)
continue;
set_speed (both_speeds, arg, mode);
*speed_was_set = true;
*require_set_attr = true;
}
else
{
if (! recover_mode (arg, mode))
{
error (0, 0, _("invalid argument %s"), quote (arg));
usage (EXIT_FAILURE);
}
*require_set_attr = true;
}
}
}
}
int
main (int argc, char **argv)
{
@ -1092,7 +1292,6 @@ main (int argc, char **argv)
bool speed_was_set _GL_UNUSED;
bool verbose_output;
bool recoverable_output;
int k;
bool noargs = true;
char *file_name = NULL;
const char *device_name;
@ -1179,15 +1378,18 @@ main (int argc, char **argv)
die (EXIT_FAILURE, 0,
_("when specifying an output style, modes may not be set"));
/* FIXME: it'd be better not to open the file until we've verified
that all arguments are valid. Otherwise, we could end up doing
only some of the requested operations and then failing, probably
leaving things in an undesirable state. */
device_name = file_name ? file_name : _("standard input");
if (!noargs && !verbose_output && !recoverable_output)
{
static struct termios check_mode;
apply_settings (/* checking= */ true, device_name, argv, argc,
&check_mode, &speed_was_set, &require_set_attr);
}
if (file_name)
{
int fdflags;
device_name = file_name;
if (fd_reopen (STDIN_FILENO, device_name, O_RDONLY | O_NONBLOCK, 0) < 0)
die (EXIT_FAILURE, errno, "%s", quotef (device_name));
if ((fdflags = fcntl (STDIN_FILENO, F_GETFL)) == -1
@ -1195,8 +1397,6 @@ main (int argc, char **argv)
die (EXIT_FAILURE, errno, _("%s: couldn't reset non-blocking mode"),
quotef (device_name));
}
else
device_name = _("standard input");
if (tcgetattr (STDIN_FILENO, &mode))
die (EXIT_FAILURE, errno, "%s", quotef (device_name));
@ -1211,174 +1411,8 @@ main (int argc, char **argv)
speed_was_set = false;
require_set_attr = false;
for (k = 1; k < argc; k++)
{
char const *arg = argv[k];
bool match_found = false;
bool not_set_attr = false;
bool reversed = false;
int i;
if (! arg)
continue;
if (arg[0] == '-')
{
++arg;
reversed = true;
}
if (STREQ (arg, "drain"))
{
tcsetattr_options = reversed ? TCSANOW : TCSADRAIN;
continue;
}
for (i = 0; mode_info[i].name != NULL; ++i)
{
if (STREQ (arg, mode_info[i].name))
{
if ((mode_info[i].flags & NO_SETATTR) == 0)
{
match_found = set_mode (&mode_info[i], reversed, &mode);
require_set_attr = true;
}
else
match_found = not_set_attr = true;
break;
}
}
if (!match_found && reversed)
{
error (0, 0, _("invalid argument %s"), quote (arg - 1));
usage (EXIT_FAILURE);
}
if (!match_found)
{
for (i = 0; control_info[i].name != NULL; ++i)
{
if (STREQ (arg, control_info[i].name))
{
if (k == argc - 1)
{
error (0, 0, _("missing argument to %s"), quote (arg));
usage (EXIT_FAILURE);
}
match_found = true;
++k;
set_control_char (&control_info[i], argv[k], &mode);
require_set_attr = true;
break;
}
}
}
if (!match_found || not_set_attr)
{
if (STREQ (arg, "ispeed"))
{
if (k == argc - 1)
{
error (0, 0, _("missing argument to %s"), quote (arg));
usage (EXIT_FAILURE);
}
++k;
set_speed (input_speed, argv[k], &mode);
speed_was_set = true;
require_set_attr = true;
}
else if (STREQ (arg, "ospeed"))
{
if (k == argc - 1)
{
error (0, 0, _("missing argument to %s"), quote (arg));
usage (EXIT_FAILURE);
}
++k;
set_speed (output_speed, argv[k], &mode);
speed_was_set = true;
require_set_attr = true;
}
#ifdef TIOCEXT
/* This is the BSD interface to "extproc".
Even though it's an lflag, an ioctl is used to set it. */
else if (STREQ (arg, "extproc"))
{
int val = ! reversed;
if (ioctl (STDIN_FILENO, TIOCEXT, &val) != 0)
{
die (EXIT_FAILURE, errno, _("%s: error setting %s"),
quotef_n (0, device_name), quote_n (1, arg));
}
}
#endif
#ifdef TIOCGWINSZ
else if (STREQ (arg, "rows"))
{
if (k == argc - 1)
{
error (0, 0, _("missing argument to %s"), quote (arg));
usage (EXIT_FAILURE);
}
++k;
set_window_size (integer_arg (argv[k], INT_MAX), -1,
device_name);
}
else if (STREQ (arg, "cols")
|| STREQ (arg, "columns"))
{
if (k == argc - 1)
{
error (0, 0, _("missing argument to %s"), quote (arg));
usage (EXIT_FAILURE);
}
++k;
set_window_size (-1, integer_arg (argv[k], INT_MAX),
device_name);
}
else if (STREQ (arg, "size"))
{
max_col = screen_columns ();
current_col = 0;
display_window_size (false, device_name);
}
#endif
#ifdef HAVE_C_LINE
else if (STREQ (arg, "line"))
{
unsigned long int value;
if (k == argc - 1)
{
error (0, 0, _("missing argument to %s"), quote (arg));
usage (EXIT_FAILURE);
}
++k;
mode.c_line = value = integer_arg (argv[k], ULONG_MAX);
if (mode.c_line != value)
error (0, 0, _("invalid line discipline %s"), quote (argv[k]));
require_set_attr = true;
}
#endif
else if (STREQ (arg, "speed"))
{
max_col = screen_columns ();
display_speed (&mode, false);
}
else if (string_to_baud (arg) != (speed_t) -1)
{
set_speed (both_speeds, arg, &mode);
speed_was_set = true;
require_set_attr = true;
}
else
{
if (! recover_mode (arg, &mode))
{
error (0, 0, _("invalid argument %s"), quote (arg));
usage (EXIT_FAILURE);
}
require_set_attr = true;
}
}
}
apply_settings (/* checking= */ false, device_name, argv, argc,
&mode, &speed_was_set, &require_set_attr);
if (require_set_attr)
{

View File

@ -22,6 +22,7 @@ print_ver_ stty
require_controlling_input_terminal_
require_trap_signame_
require_strace_ ioctl
trap '' TTOU # Ignore SIGTTOU
@ -81,4 +82,11 @@ done
stty $(cat $saved_state)
# Ensure we validate options before accessing the device
strace -o log1 -e ioctl stty --version || fail=1
n_ioctl1=$(wc -l < log1) || framework_failure_
returns_ 1 strace -o log2 -e ioctl stty -blahblah || fail=1
n_ioctl2=$(wc -l < log2) || framework_failure_
test "$n_ioctl1" = "$n_ioctl2" || fail=1
Exit $fail