Commit: patch 9.2.1113: use-after-free and memory leak in serverSendToVim()

1 view
Skip to first unread message

Christian Brabandt

unread,
Sep 16, 2026, 2:15:17 PM (5 days ago) Sep 16
to vim...@googlegroups.com
patch 9.2.1113: use-after-free and memory leak in serverSendToVim()

Commit: https://github.com/vim/vim/commit/f6eaad742e7abb2ef2428a589f981eef6c119954
Author: dyingc <dyi...@hotmail.com>
Date: Wed Sep 16 18:03:52 2026 +0000

patch 9.2.1113: use-after-free and memory leak in serverSendToVim()

Problem: LookupName() stores the loose name without checking whether the
window id parsed -- the sscanf() return value is discarded -- so
an id that is not hex, or is zero, leaves the string allocated
while the function returns None, and serverSendToVim() returns
without freeing it. The retry path frees "loosename" without
clearing it, so if the next round finds an exact match the stale
pointer is passed back to LookupName() as the name to delete,
read in STRICMP(), and then freed a second time, which aborts
the process.
Solution: Only keep the loose name when the id parsed to a usable window,
and clear "loosename" on the retry path instead of only freeing
it. Add a test that walks the leaking path, so the ASan CI
catches a regression.

closes: #21320

Signed-off-by: dyingc <dyi...@hotmail.com>
Signed-off-by: Christian Brabandt <c...@256bit.org>

diff --git a/src/if_xcmdsrv.c b/src/if_xcmdsrv.c
index 22423a2ed..2ef627fd6 100644
--- a/src/if_xcmdsrv.c
+++ b/src/if_xcmdsrv.c
@@ -421,7 +421,7 @@ serverSendToVim(
{
LookupName(dpy, loosename ? loosename : name,
/*DELETE=*/TRUE, NULL);
- vim_free(loosename);
+ VIM_CLEAR(loosename);
continue;
}
}
@@ -952,8 +952,9 @@ LookupName(
if (*p != 0 && IsSerialName(p + 1)
&& STRNICMP(name, p + 1, STRLEN(name)) == 0)
{
- sscanf((char *)entry, "%x", &returnValue);
- *loose = vim_strsave(p + 1);
+ if (sscanf((char *)entry, "%x", &returnValue) == 1
+ && returnValue != (int_u)None)
+ *loose = vim_strsave(p + 1);
break;
}
while (*p != 0)
diff --git a/src/testdir/test_clientserver.vim b/src/testdir/test_clientserver.vim
index 17ea44d41..f6190ae5c 100644
--- a/src/testdir/test_clientserver.vim
+++ b/src/testdir/test_clientserver.vim
@@ -243,6 +243,48 @@ func Test_client_server_stopinsert()
endtry
endfunc

+" A "VimRegistry" entry whose window id does not parse makes LookupName() keep
+" the loose name it allocated while still returning None. Nothing observable
+" differs, so this test asserts nothing about the leak itself: it walks the
+" path so that the ASan job notices if it comes back.
+func Test_clientserver_x11_registry_loose_name()
+ let g:test_is_flaky = 1
+ CheckFeature x11
+ if !executable('xprop')
+ throw 'Skipped: xprop is not available'
+ endif
+ " Only the x11 backend goes through serverSendToVim().
+ if $VIM_CLIENTSERVER != '' && $VIM_CLIENTSERVER !=? 'x11'
+ throw 'Skipped: the clientserver backend is not x11'
+ endif
+ let cmd = GetVimCommand()
+ if cmd == ''
+ throw 'GetVimCommand() failed'
+ endif
+ call Check_X11_Connection()
+
+ " Keep a client on the display: when the last one disconnects the X server
+ " may reset and drop the property before Vim can read it.
+ let name = 'XVIMTESTREG'
+ let job = job_start(cmd .. ' --servername ' .. name,
+ \ {'stoponexit': 'kill', 'out_io': 'null'})
+ call WaitForAssert({-> assert_equal("run", job_status(job))})
+ call WaitForAssert({-> assert_match(name, serverlist())})
+
+ " Replaces the whole registry, so this unregisters the server above too.
+ call system("xprop -root -f VimRegistry 8s -set VimRegistry 'zz " .. name .. "1'")
+ call assert_equal(0, v:shell_error)
+
+ " Loose match: "XVIMTESTREG" against a registered "XVIMTESTREG1" whose
+ " window id is not hex.
+ call system(cmd .. ' --servername ' .. name .. ' --remote-send x')
+ call assert_equal(0, v:shell_error)
+
+ call system('xprop -root -remove VimRegistry')
+ call job_stop(job, 'kill')
+ call WaitForAssert({-> assert_equal("dead", job_status(job))})
+endfunc
+
" Test if socket server, X11, and mswin backends can be chosen and work properly.
func Test_client_server_multiple_backends()
CheckFeature socketserver
diff --git a/src/version.c b/src/version.c
index 8f76f2d83..6f59b70b9 100644
--- a/src/version.c
+++ b/src/version.c
@@ -763,6 +763,8 @@ static char *(features[]) =

static int included_patches[] =
{ /* Add new patch number below this line */
+/**/
+ 1113,
/**/
1112,
/**/
Reply all
Reply to author
Forward
0 new messages