[newlib-cygwin] Cygwin: pty: validate the cursor-position reply before moving the pcon cursor
Takashi Yano via Cygwin-cvs <[email protected]> Tue, 30 Jun 2026 08:34:48 +0000 (GMT)
| Newsgroups | gmane.os.cygwin.cvs |
|---|---|
| Message-ID | <[email protected]> |
https://sourceware.org/git/gitweb.cgi?p=3Dnewlib-cygwin.git;h=3Da430b94d94b= 8ef227ebc8ee9f243b1166427b11c commit a430b94d94b8ef227ebc8ee9f243b1166427b11c Author: Johannes Schindelin <[email protected]> Date: Thu Jun 25 13:41:44 2026 +0200 Cygwin: pty: validate the cursor-position reply before moving the pcon = cursor =20 The CSI6n reply handler added in "Cygwin: pty: Fixup pty state after a cygwin app exits" runs sscanf() on the terminal's response but ignores its return value, so a malformed or partial reply leaves the x and y locals uninitialised and hands them to SetConsoleCursorPosition(), which is exactly the cursor corruption the commit set out to prevent. =20 Only call the fixup when sscanf() reports both coordinates parsed, and in fixup_pcon_cursor_position() clamp the coordinates into the valid SHORT range before the COORD cast so a stray reply cannot wrap into a negative position. While there, check OpenProcess() for NULL (the nat-pipe owner may have exited) and check the DuplicateHandle() result instead of using a possibly-NULL screen-buffer handle. =20 Fixes: b34394d456b6 ("Cygwin: pty: Fixup pty state after a cygwin app e= xits") Assisted-by: Opus 4.8 Signed-off-by: Johannes Schindelin <[email protected]> Reviewed-by: Takashi Yano <[email protected]> Diff: --- winsup/cygwin/fhandler/pty.cc | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/winsup/cygwin/fhandler/pty.cc b/winsup/cygwin/fhandler/pty.cc index 847e5a082..c1de25baa 100644 --- a/winsup/cygwin/fhandler/pty.cc +++ b/winsup/cygwin/fhandler/pty.cc @@ -418,12 +418,23 @@ fhandler_pty_slave::req_fixup_pcon_state (void) void fhandler_pty_master::fixup_pcon_cursor_position (int x, int y) { + /* A malformed or out-of-range reply must not be turned into a wrapped + negative COORD. */ + if (x < 1 || y < 1 || x > 0x7fff || y > 0x7fff) + return; HANDLE pcon_owner =3D OpenProcess (PROCESS_DUP_HANDLE, FALSE, get_ttyp ()->nat_pipe_owner_pid); + if (!pcon_owner) + /* The nat-pipe owner is gone; nothing to sync to. */ + return; HANDLE h_pcon_out =3D NULL; - DuplicateHandle (pcon_owner, get_ttyp ()->h_pcon_out, - GetCurrentProcess (), &h_pcon_out, - 0, TRUE, DUPLICATE_SAME_ACCESS); + if (!DuplicateHandle (pcon_owner, get_ttyp ()->h_pcon_out, + GetCurrentProcess (), &h_pcon_out, + 0, TRUE, DUPLICATE_SAME_ACCESS)) + { + CloseHandle (pcon_owner); + return; + } CloseHandle (pcon_owner); DWORD target_pid =3D get_ttyp ()->nat_pipe_owner_pid; DWORD resume_pid =3D @@ -2580,8 +2591,8 @@ fhandler_pty_master::write (const void *ptr, size_t l= en) if (get_ttyp ()->req_fixup_pcon_cur_pos) { int x, y; - sscanf (wpbuf, "\033[%d;%dR", &y, &x); - fixup_pcon_cursor_position (x, y); + if (sscanf (wpbuf, "\033[%d;%dR", &y, &x) =3D=3D 2) + fixup_pcon_cursor_position (x, y); get_ttyp ()->req_fixup_pcon_cur_pos =3D false; } else if (!get_ttyp ()->req_xfer_input)