lib/56173: libcurses: reset_prog_mode() after reset_shell_mode() overwrites original TTY state

[mforney%mforney.org@localhost]

>Number:         56173
>Category:       lib
>Synopsis:       libcurses: reset_prog_mode() after reset_shell_mode() overwrites original TTY state
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    lib-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sat May 15 04:15:00 +0000 2021
>Originator:     Michael Forney
>Release:        trunk
>Organization:
>Environment:
>Description:
If a curses program calls reset_shell_mode() and then reset_prog_mode() to temporarily switch to shell mode and then back to curses mode, the original TTY settings are overwritten. This results in incorrect TTY settings after the application exits.

This pattern is used in the text editor vis when you read output from a shell command.

The problem seems to be that reset_shell_mode() calls __stopwin() to save the curses mode settings and restore the shell mode settings, but reset_prog_mode() just restores the saved curses mode settings, leaving the screen in the "endwin" state. Then, when the application performs a refresh(), __restartwin() is called which saves the current (already restored) settings as the original settings, and then restores the curses mode settings again. At this point, the original settings have been overwritten with the curses mode settings, which persist even after the application calls endwin() and exits.

In other words, the following sequence of events occurs:

	reset_shell_mode()
		__stopwin()
			saved settings = current settings
			current settings = original settings
			endwin = 1
	reset_prog_mode()
		current settings = saved settings
	refresh()
		__restartwin()
			original settings = current settings
			current settings = saved settings

At this point we have original settings == current settings == curses mode settings. The original settings have been overwritten.

It appears that this behavior dates back to this commit from 2003:
https://github.com/NetBSD/src/commit/880234af7eb36a86d6adfa775327ccec592d519d#diff-3825aa201b54dbfe36d66fe3f650737571d39e3459b936691d1c955f37871d2dL199-R204

Previously, reset_shell_mode() and reset_prog_mode() were symmetrical, calling __endwin() and __restartwin() respectively. After the change, reset_shell_mode() still calls __endwin(), but reset_prog_mode() just changes the TTY settings leaving the screen in the endwin state.
>How-To-Repeat:
1. Build vis (https://github.com/martanne/vis) against NetBSD libcurses, then run it.
2. Type ":<echo hello" to read the output of a command into the buffer.
3. Exit vis with ":q!".
4. Terminal settings are now messed up.
>Fix:
I don't quite understand what problem the commit referenced above was attempting to fix, but if I revert the highlighted hunk, the problem is fixed.

diff --git a/lib/libcurses/tstp.c b/lib/libcurses/tstp.c
index f0baa999120b..ef54143a4a54 100644
--- a/lib/libcurses/tstp.c
+++ b/lib/libcurses/tstp.c
@@ -348,8 +348,8 @@ int
 reset_prog_mode(void)
 {
 
-	return tcsetattr(fileno(_cursesi_screen->infd), TCSASOFT | TCSADRAIN,
-			 &_cursesi_screen->save_termios) ? ERR : OK;
+	__restartwin();
+	return OK;
 }
 
 int