[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)