Subject: Re: bug#28859: Segmentation fault with NULL pointer dereference in 'stty'

From: Pádraig Brady <[email-masked]>
Date: Mon, 16 Oct 2017 02:30:56 -0700

On 15/10/17 18:07, Jaeseung Choi wrote:
> Dear GNU team,
> 
> While testing coreutils for a research purpose, we found the following
> crash in 'stty'. Running stty with the command-line "stty eol -F AA"
> raises a crash as below. We did not change any terminal setting, and
> believe the bug is irrelevant from any specific terminal
> configuration.
> 
> jason@ubuntu:~$ tar -xf coreutils-8.28.tar.xz
> jason@ubuntu:~$ cd coreutils-8.28/
> jason@ubuntu:~/coreutils-8.28$ mkdir obj
> jason@ubuntu:~/coreutils-8.28$ cd obj
> jason@ubuntu:~/coreutils-8.28/obj$ ../configure --disable-nls && make
> ...
> jason@ubuntu:~/coreutils-8.28/obj$ gdb ./src/stty -q
> Reading symbols from ./src/stty...done.
> (gdb) run eol -F AA
> Starting program: /home/jason/coreutils-8.28/obj/src/stty eol -F AA
> 
> Program received signal SIGSEGV, Segmentation fault.
> set_control_char (info=0x40a6f8 <control_info+120>, info=0x40a6f8
> <control_info+120>, mode=0x6103c0 <check_mode>, arg=0x0) at
> ../src/stty.c:1695
> 1695      else if (arg[0] == '\0' || arg[1] == '\0')
> (gdb) x/i $rip
> => 0x40387a <apply_settings+746>:       movzbl (%rbx),%r14d
> (gdb) info reg rbx
> rbx            0x0      0
> (gdb)
> 
> We could reproduce the bug in coreutils from version 8.27 to 8.28.
> Also, the bug was reproducible in both Ubuntu 16.04 and Debian 9.1.
> But the stty program pre-built in Debian 9.1 did not crash because
> currently 8.26 version is installed in Debian.

This is actually an old bug which you can reproduce with -F /dev/tty.
The attached should fix it up.

thanks!
Pádraig

>From 5e74a57e1b1491ab3a66dc6b6cf6b6d3ae36a138 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <[email-masked]>
Date: Mon, 16 Oct 2017 02:17:34 -0700
Subject: [PATCH] stty: fix processing of options when -F is specified

* src/stty.c (main): Pass argi+1 so that already processed options
are not considered in the argument count.  This is significant
when -F is specified.
* NEWS: Mention the fix.
* tests/misc/stty-invalid.sh: Add a test case.
Fixes https://bugs.gnu.org/28859
---
 NEWS                       | 3 +++
 src/stty.c                 | 4 ++--
 tests/misc/stty-invalid.sh | 6 ++++++
 3 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/NEWS b/NEWS
index 2878b70..13a3ee0 100644
--- a/NEWS
+++ b/NEWS
@@ -11,6 +11,9 @@ GNU coreutils NEWS                                    -*- outline -*-
   to attempt to hide the original length of the file name.
   [bug introduced in coreutils-8.28]
 
+  stty no longer crashes when processing settings with -F also specified.
+  [bug instroduced in coreutils-5.3.0]
+
 ** Build-related
 
   Default man pages are now distributed which are used if perl is
diff --git a/src/stty.c b/src/stty.c
index 48aac59..8957595 100644
--- a/src/stty.c
+++ b/src/stty.c
@@ -1383,7 +1383,7 @@ main (int argc, char **argv)
   if (!noargs && !verbose_output && !recoverable_output)
     {
       static struct termios check_mode;
-      apply_settings (/* checking= */ true, device_name, argv, argc,
+      apply_settings (/* checking= */ true, device_name, argv, argi + 1,
                       &check_mode, &speed_was_set, &require_set_attr);
     }
 
@@ -1411,7 +1411,7 @@ main (int argc, char **argv)
 
   speed_was_set = false;
   require_set_attr = false;
-  apply_settings (/* checking= */ false, device_name, argv, argc,
+  apply_settings (/* checking= */ false, device_name, argv, argi + 1,
                   &mode, &speed_was_set, &require_set_attr);
 
   if (require_set_attr)
diff --git a/tests/misc/stty-invalid.sh b/tests/misc/stty-invalid.sh
index 06186e9..7402c93 100755
--- a/tests/misc/stty-invalid.sh
+++ b/tests/misc/stty-invalid.sh
@@ -41,6 +41,12 @@ returns_ 1 stty $(echo $saved_state |sed 's/^[^:]*:/'$hex_2_64:/) \
 returns_ 1 stty $(echo $saved_state |sed 's/:[0-9a-f]*$/:'$hex_2_64/) \
   2>/dev/null || fail=1
 
+# From coreutils 5.3.0 to 8.28, the following would crash
+# due to incorrect argument handling.
+if tty -s </dev/tty; then
+  returns_ 1 stty eol -F /dev/tty || fail=1
+fi
+
 # 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
-- 
2.9.3