Skip to content

Commit a430b94

Browse files
dschotyan0
authored andcommitted
Cygwin: pty: validate the cursor-position reply before moving the pcon cursor
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. 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. Fixes: b34394d ("Cygwin: pty: Fixup pty state after a cygwin app exits") Assisted-by: Opus 4.8 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Reviewed-by: Takashi Yano <takashi.yano@nifty.ne.jp>
1 parent 60a8889 commit a430b94

1 file changed

Lines changed: 16 additions & 5 deletions

File tree

winsup/cygwin/fhandler/pty.cc

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -418,12 +418,23 @@ fhandler_pty_slave::req_fixup_pcon_state (void)
418418
void
419419
fhandler_pty_master::fixup_pcon_cursor_position (int x, int y)
420420
{
421+
/* A malformed or out-of-range reply must not be turned into a wrapped
422+
negative COORD. */
423+
if (x < 1 || y < 1 || x > 0x7fff || y > 0x7fff)
424+
return;
421425
HANDLE pcon_owner = OpenProcess (PROCESS_DUP_HANDLE, FALSE,
422426
get_ttyp ()->nat_pipe_owner_pid);
427+
if (!pcon_owner)
428+
/* The nat-pipe owner is gone; nothing to sync to. */
429+
return;
423430
HANDLE h_pcon_out = NULL;
424-
DuplicateHandle (pcon_owner, get_ttyp ()->h_pcon_out,
425-
GetCurrentProcess (), &h_pcon_out,
426-
0, TRUE, DUPLICATE_SAME_ACCESS);
431+
if (!DuplicateHandle (pcon_owner, get_ttyp ()->h_pcon_out,
432+
GetCurrentProcess (), &h_pcon_out,
433+
0, TRUE, DUPLICATE_SAME_ACCESS))
434+
{
435+
CloseHandle (pcon_owner);
436+
return;
437+
}
427438
CloseHandle (pcon_owner);
428439
DWORD target_pid = get_ttyp ()->nat_pipe_owner_pid;
429440
DWORD resume_pid =
@@ -2580,8 +2591,8 @@ fhandler_pty_master::write (const void *ptr, size_t len)
25802591
if (get_ttyp ()->req_fixup_pcon_cur_pos)
25812592
{
25822593
int x, y;
2583-
sscanf (wpbuf, "\033[%d;%dR", &y, &x);
2584-
fixup_pcon_cursor_position (x, y);
2594+
if (sscanf (wpbuf, "\033[%d;%dR", &y, &x) == 2)
2595+
fixup_pcon_cursor_position (x, y);
25852596
get_ttyp ()->req_fixup_pcon_cur_pos = false;
25862597
}
25872598
else if (!get_ttyp ()->req_xfer_input)

0 commit comments

Comments
 (0)