From 02599650b1ce9e53c837c29db5002e07337a20ab Mon Sep 17 00:00:00 2001 From: Jim Meyering Date: Sun, 20 May 2007 14:45:38 +0200 Subject: [PATCH] stty: diagnose an invalid hex value in 35-colon commmand-line argument * NEWS: Mention this. * src/stty.c (strtoul_tcflag_t, strtoul_cc_t): New functions. (recover_mode): Use those functions (not sscanf), to parse the string robustly. * tests/stty/invalid: New file. Test for the above. * tests/stty/Makefile.am (TESTS): Add invalid. * .x-sc_prohibit_atoi_atof: Don't exempt stty.c from this check. Add tests/stty/invalid so we don't have to obfuscate the comment about sscanf therein. * Makefile.maint (sc_prohibit_atoi_atof): Mention sscanf in the diagnostic, too. --- .x-sc_prohibit_atoi_atof | 2 +- ChangeLog | 13 +++++++++ Makefile.maint | 2 +- NEWS | 5 ++++ src/stty.c | 75 ++++++++++++++++++++++++++++++------------------ tests/stty/Makefile.am | 2 +- tests/stty/invalid | 64 +++++++++++++++++++++++++++++++++++++++++ 7 files changed, 132 insertions(+), 31 deletions(-) create mode 100755 tests/stty/invalid diff --git a/.x-sc_prohibit_atoi_atof b/.x-sc_prohibit_atoi_atof index d995223..c7b8603 100644 --- a/.x-sc_prohibit_atoi_atof +++ b/.x-sc_prohibit_atoi_atof @@ -8,4 +8,4 @@ ChangeLog ^lib/group-member\.c$ ^Makefile\.maint$ ^doc/coreutils.texi$ -^src/stty.c$ +^tests/stty/invalid$ diff --git a/ChangeLog b/ChangeLog index dc2870b..0a2f553 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,18 @@ 2007-05-20 Jim Meyering + stty: diagnose an invalid hex value in 35-colon commmand-line argument + * NEWS: Mention this. + * src/stty.c (strtoul_tcflag_t, strtoul_cc_t): New functions. + (recover_mode): Use those functions (not sscanf), to parse the + string robustly. + * tests/stty/invalid: New file. Test for the above. + * tests/stty/Makefile.am (TESTS): Add invalid. + * .x-sc_prohibit_atoi_atof: Don't exempt stty.c from this check. + Add tests/stty/invalid so we don't have to obfuscate the comment + about sscanf therein. + * Makefile.maint (sc_prohibit_atoi_atof): Mention sscanf in the + diagnostic, too. + * TODO: Remove some now-completed or no longer relevant items. 2007-05-19 Jim Meyering diff --git a/Makefile.maint b/Makefile.maint index 99e4b5c..795cd99 100644 --- a/Makefile.maint +++ b/Makefile.maint @@ -122,7 +122,7 @@ sc_space_tab: # Instead, use strto* functions. sc_prohibit_atoi_atof: @grep -nE '\<([fs]?scanf|ato([filq]|ll))\>' $$($(CVS_LIST_EXCEPT)) && \ - { echo '$(ME): do not use *scan''f, ato''f, ato''i, ato''l, ato''ll, or ato''q' \ + { echo '$(ME): do not use *scan''f, ato''f, ato''i, ato''l, ato''ll, ato''q, or ss''canf' \ 1>&2; exit 1; } || : # Using EXIT_SUCCESS as the first argument to error is misleading, diff --git a/NEWS b/NEWS index 2a23108..0000a2b 100644 --- a/NEWS +++ b/NEWS @@ -30,6 +30,11 @@ GNU coreutils NEWS -*- outline -*- tr -c no longer aborts when translating with Set2 larger than the complement of Set1. [introduced with the original version, in 1992] +** Improved robustness + + stty no longer silently accepts certain invalid hex values + in its 35-colon commmand-line argument + * Noteworthy changes in release 6.9 (2007-03-22) [stable] diff --git a/src/stty.c b/src/stty.c index ce0cb0c..eb26907 100644 --- a/src/stty.c +++ b/src/stty.c @@ -1654,42 +1654,61 @@ display_recoverable (struct termios *mode) putchar ('\n'); } +/* NOTE: identical to below, modulo use of tcflag_t */ +static int +strtoul_tcflag_t (char const *s, int base, char **p, tcflag_t *result, + char delim) +{ + unsigned long ul; + errno = 0; + ul = strtoul (s, p, base); + if (errno || **p != delim || *p == s || (tcflag_t) ul != ul) + return -1; + *result = ul; + return 0; +} + +/* NOTE: identical to above, modulo use of cc_t */ +static int +strtoul_cc_t (char const *s, int base, char **p, cc_t *result, char delim) +{ + unsigned long ul; + errno = 0; + ul = strtoul (s, p, base); + if (errno || **p != delim || *p == s || (cc_t) ul != ul) + return -1; + *result = ul; + return 0; +} + +/* Parse the output of display_recoverable. + Return false if any part of it is invalid. */ static bool recover_mode (char const *arg, struct termios *mode) { + tcflag_t flag[4]; + char const *s = arg; size_t i; - int n; - unsigned long int chr; - unsigned long int iflag, oflag, cflag, lflag; - - /* Scan into temporaries since it is too much trouble to figure out - the right format for `tcflag_t'. */ - if (sscanf (arg, "%lx:%lx:%lx:%lx%n", - &iflag, &oflag, &cflag, &lflag, &n) != 4) - return false; - mode->c_iflag = iflag; - mode->c_oflag = oflag; - mode->c_cflag = cflag; - mode->c_lflag = lflag; - if (mode->c_iflag != iflag - || mode->c_oflag != oflag - || mode->c_cflag != cflag - || mode->c_lflag != lflag) - return false; - arg += n; - for (i = 0; i < NCCS; ++i) + for (i = 0; i < 4; i++) { - if (sscanf (arg, ":%lx%n", &chr, &n) != 1) + char *p; + if (strtoul_tcflag_t (s, 16, &p, flag + i, ':') != 0) return false; - mode->c_cc[i] = chr; - if (mode->c_cc[i] != chr) - return false; - arg += n; + s = p + 1; } + mode->c_iflag = flag[0]; + mode->c_oflag = flag[1]; + mode->c_cflag = flag[2]; + mode->c_lflag = flag[3]; - /* Fail if there are too many fields. */ - if (*arg != '\0') - return false; + for (i = 0; i < NCCS; ++i) + { + char *p; + char delim = i < NCCS - 1 ? ':' : '\0'; + if (strtoul_cc_t (s, 16, &p, mode->c_cc + i, delim) != 0) + return false; + s = p + 1; + } return true; } diff --git a/tests/stty/Makefile.am b/tests/stty/Makefile.am index 5cd33b5..b3c460e 100644 --- a/tests/stty/Makefile.am +++ b/tests/stty/Makefile.am @@ -1,5 +1,5 @@ ## Process this file with automake to produce Makefile.in -*-Makefile-*-. -TESTS = row-col-1 basic-1 +TESTS = invalid row-col-1 basic-1 EXTRA_DIST = $(TESTS) TESTS_ENVIRONMENT = \ CU_TEST_NAME=`basename $(abs_srcdir)`,$$tst \ diff --git a/tests/stty/invalid b/tests/stty/invalid new file mode 100755 index 0000000..127a2f5 --- /dev/null +++ b/tests/stty/invalid @@ -0,0 +1,64 @@ +#!/bin/sh +# Ensure that stty diagnoses invalid inputs, rather than silently misbehaving. + +# Copyright (C) 2007 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. + +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. + +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA +# 02110-1301, USA. + +if test "$VERBOSE" = yes; then + set -x + stty --version +fi + +# Make sure there's a tty on stdin. +. $srcdir/../input-tty + +pwd=`pwd` +t0=`echo "$0"|sed 's,.*/,,'`.tmp; tmp=$t0/$$ +trap 'status=$?; cd "$pwd" && chmod -R u+rwx $t0 && rm -rf $t0 && exit $status' 0 +trap '(exit $?); exit $?' 1 2 13 15 + +framework_failure=0 +mkdir -p $tmp || framework_failure=1 +cd $tmp || framework_failure=1 + +if test $framework_failure = 1; then + echo "$0: failure in testing framework" 1>&2 + (exit 1); exit 1 +fi + +fail=0 + +saved_state=`stty -g` || fail=1 +stty $saved_state || fail=1 + +# Before coreutils-6.10, if stty were given an argument with 35 colons +# separating 36 hexadecimal strings, stty would fail to diagnose as invalid +# any number that was out of range as long as sscanf happened to +# overflow/wrap it back into the range of the corresponding type (either +# tcflag_t or cc_t). + +# For each of the following, before 6.10, stty would fail to +# diagnose the error on at least Solaris 10. +hex_2_64=10000000000000000 +stty `echo $saved_state |sed 's/^[^:]*:/'$hex_2_64:/` 2>/dev/null && fail=1 +stty `echo $saved_state |sed 's/:[0-9a-f]*$/:'$hex_2_64/` 2>/dev/null && fail=1 + +# Just in case either of the above mistakenly succeeds (and changes +# the state of our tty), try to restore the initial state. +stty $saved_state || fail=1 + +(exit $fail); exit $fail -- 2.7.4