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

From: Jim Meyering <[email-masked]>
Date: Tue, 17 Oct 2017 11:12:48 -0700

On Tue, Oct 17, 2017 at 12:37 AM, Pádraig Brady <[email-masked]> wrote:
> On 16/10/17 10:49, Jim Meyering wrote:
>> On Mon, Oct 16, 2017 at 2:30 AM, Pádraig Brady <[email-masked]> wrote:
>>> 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.
>>
>> Thank you!
>> If it's not too hard to determine, would you please mention in the log
>> the commit that introduced the bug?
>
> Updated patch attached. I mistakenly thought getopt would
> permute the argv so NULLs were at the end.  The attached
> caters for NULLs interspersed in the argv[].

Good catch!
One suggestion: indent the backslashes to column 72, e.g., with this patch:
diff --git a/src/stty.c b/src/stty.c
index 1a5c1e962..29f78375a 100644
--- a/src/stty.c
+++ b/src/stty.c
@@ -1089,11 +1089,11 @@ apply_settings (bool checking, const char *device_name,
                 struct termios *mode, bool *speed_was_set,
                 bool *require_set_attr)
 {
-#define check_argument(arg) \
-  if (k == n_settings - 1 || ! settings[k+1]) \
-    { \
-      error (0, 0, _("missing argument to %s"), quote (arg)); \
-      usage (EXIT_FAILURE); \
+#define check_argument(arg)						\
+  if (k == n_settings - 1 || ! settings[k+1])				\
+    {									\
+      error (0, 0, _("missing argument to %s"), quote (arg));		\
+      usage (EXIT_FAILURE);						\
     }

   for (int k = 1; k < n_settings; k++)