From: Yasuhiro Matsumoto Date: Thu, 23 Jul 2026 20:50:10 +0000 (+0000) Subject: patch 9.2.0842: [security]: stack buffer overflow in socket server X-Git-Tag: v9.2.0842^0 X-Git-Url: http://git.ipfire.org/gitweb/index.cgi?a=commitdiff_plain;h=5598618b2daf8e36b3bf0251caaefcf0bf8e85e4;p=thirdparty%2Fvim.git patch 9.2.0842: [security]: stack buffer overflow in socket server Problem: [security]: stack buffer overflow in socket server (tdjackey) Solution: Cap accepted socketserver clients (Yasuhiro Matsumoto) Github Security Advisory: https://github.com/vim/vim/security/advisories/GHSA-49m8-wwxj-mr69 Signed-off-by: Yasuhiro Matsumoto Signed-off-by: Christian Brabandt --- diff --git a/src/channel.c b/src/channel.c index d7fa3bd2c7..0519a157be 100644 --- a/src/channel.c +++ b/src/channel.c @@ -4028,6 +4028,9 @@ channel_fill_wfds(int maxfd_arg, fd_set *wfds) chanpart_T *in_part = &ch->ch_part[PART_IN]; if (in_part->ch_fd != INVALID_FD +# ifdef FD_SETSIZE + && (int)in_part->ch_fd < FD_SETSIZE +# endif && is_channel_write_remaining(in_part)) { FD_SET((int)in_part->ch_fd, wfds); @@ -4165,7 +4168,7 @@ channel_wait(channel_T *channel, sock_T fd, int timeout) #else for (;;) { - struct pollfd fds[MAX_OPEN_CHANNELS + 1]; + struct pollfd fds[MAX_OPEN_CHANNELS + 1 + MAX_CLIENT_CHANNELS]; int nfd = 1; fds[0].fd = fd; @@ -5415,6 +5418,12 @@ channel_select_setup( } else { +# ifdef FD_SETSIZE + // An fd that does not fit in the fd_set cannot be watched + // with select(); skip it rather than overflow the set. + if ((int)fd >= FD_SETSIZE) + continue; +# endif FD_SET((int)fd, rfds); if (maxfd < (int)fd) maxfd = (int)fd; @@ -5447,7 +5456,11 @@ channel_select_check(int ret_in, void *rfds_in, void *wfds_in) { sock_T fd = channel->ch_part[part].ch_fd; - if (ret > 0 && fd != INVALID_FD && FD_ISSET(fd, rfds)) + if (ret > 0 && fd != INVALID_FD +# ifdef FD_SETSIZE + && (int)fd < FD_SETSIZE +# endif + && FD_ISSET(fd, rfds)) { channel_read(channel, part, "channel_select_check"); FD_CLR(fd, rfds); @@ -5462,6 +5475,9 @@ channel_select_check(int ret_in, void *rfds_in, void *wfds_in) in_part = &channel->ch_part[PART_IN]; if (ret > 0 && in_part->ch_fd != INVALID_FD +# ifdef FD_SETSIZE + && (int)in_part->ch_fd < FD_SETSIZE +# endif && FD_ISSET(in_part->ch_fd, wfds)) { // Clear the flag first, ch_fd may change in channel_write_input(). diff --git a/src/os_unix.c b/src/os_unix.c index 904421772e..85b4353c43 100644 --- a/src/os_unix.c +++ b/src/os_unix.c @@ -6798,7 +6798,7 @@ RealWaitForChar(int fd, long msec, int *check_for_gpm UNUSED, int *interrupted) # endif # ifndef HAVE_SELECT // each channel may use in, out and err - struct pollfd fds[7 + 3 * MAX_OPEN_CHANNELS]; + struct pollfd fds[7 + 3 * MAX_OPEN_CHANNELS + 2 * MAX_CLIENT_CHANNELS]; int nfd; # ifdef FEAT_WAYLAND_CLIPBOARD int wayland_idx = -1; diff --git a/src/socketserver.c b/src/socketserver.c index 44107bd4a3..e693123e13 100644 --- a/src/socketserver.c +++ b/src/socketserver.c @@ -57,6 +57,7 @@ static char_u *server_addr_cwd = NULL; // CWD when server was started, used static garray_T server_replies; static channel_T *client_channels = NULL; +static int client_channel_count = 0; # define FOR_ALL_CLIENTS(ch) \ for (ch = client_channels; ch != NULL; ch = ch->ch_ss_next) @@ -538,6 +539,8 @@ socketserver_client_close(channel_T *channel) channel->ch_ss_prev->ch_ss_next = channel->ch_ss_next; if (channel->ch_ss_next != NULL) channel->ch_ss_next->ch_ss_prev = channel->ch_ss_prev; + if (client_channel_count > 0) + --client_channel_count; ch_log(NULL, "socketserver: client channel closed"); } @@ -548,6 +551,16 @@ socketserver_client_close(channel_T *channel) static void socketserver_accept(channel_T *channel) { + if (client_channel_count >= MAX_CLIENT_CHANNELS) + { + // Too many client connections: refuse this one instead of letting the + // channel's file descriptor overflow the select()/poll() sets. + ch_log(NULL, "socketserver: too many clients, refusing connection"); + channel_close(channel, FALSE); + channel_unref(channel); + return; + } + channel->ch_socketserver = true; channel->ch_ss_close_cb = socketserver_client_close; @@ -568,6 +581,7 @@ socketserver_accept(channel_T *channel) last->ch_ss_next = channel; channel->ch_ss_prev = last; } + ++client_channel_count; // We will read the command from the client later in the input loop. ch_log(NULL, "socketserver: accepted new client"); @@ -872,7 +886,11 @@ socketserver_wait(channel_T *channel, int timeout) FOR_ALL_CLIENTS(ch) { - if (ch->CH_SOCK_FD != INVALID_FD) + if (ch->CH_SOCK_FD != INVALID_FD +# ifdef FD_SETSIZE + && (int)ch->CH_SOCK_FD < FD_SETSIZE +# endif + ) { FD_SET(ch->CH_SOCK_FD, &rfds); if (maxfd < (int)ch->CH_SOCK_FD) @@ -903,6 +921,9 @@ socketserver_wait(channel_T *channel, int timeout) FOR_ALL_CLIENTS(ch) if (ch->CH_SOCK_FD != INVALID_FD +# ifdef FD_SETSIZE + && (int)ch->CH_SOCK_FD < FD_SETSIZE +# endif && FD_ISSET(ch->CH_SOCK_FD, &rfds)) channel_check(ch, PART_SOCK); @@ -920,7 +941,7 @@ socketserver_wait(channel_T *channel, int timeout) continue; } # else - struct pollfd fds[MAX_OPEN_CHANNELS + 1]; + struct pollfd fds[MAX_OPEN_CHANNELS + 1 + MAX_CLIENT_CHANNELS]; int nfd = 0; int channel_idx = -1; int server_idx = -1; diff --git a/src/testdir/test_clientserver.vim b/src/testdir/test_clientserver.vim index 3c39d01c24..d5c243fda6 100644 --- a/src/testdir/test_clientserver.vim +++ b/src/testdir/test_clientserver.vim @@ -373,6 +373,60 @@ func Test_client_server_socketserver_address() endtry endfunc +" A flood of client connections must not crash the socketserver: connections +" beyond the accept cap are refused instead of overflowing the fd_set / pollfd +" sets, and the server keeps working once they are closed again. +func Test_client_server_socketserver_connection_flood() + CheckFeature socketserver + CheckNotMSWindows + + let g:test_is_flaky = 1 + let cmd = GetVimCommand() + if cmd == '' + throw 'GetVimCommand() failed' + endif + + let actual = cmd .. ' --clientserver socket --servername channel:2001' + let job = job_start(actual, {'stoponexit': 'kill', 'out_io': 'null'}) + call WaitForAssert({-> assert_equal("run", job_status(job))}) + call WaitForAssert({-> assert_match('channel:2001', + \ system(actual .. ' --remote-expr "v:servername"'))}) + + " Open many more connections than the server accepts. + let channels = [] + for i in range(150) + let ch = ch_open('localhost:2001', {'mode': 'raw', 'waittime': 100}) + if ch_status(ch) == 'open' + call add(channels, ch) + endif + endfor + + " The server must survive, and must have refused the excess connections. + call assert_equal("run", job_status(job)) + call WaitForAssert({-> assert_inrange(1, 100, + \ len(filter(copy(channels), {_, c -> ch_status(c) == 'open'})))}) + + " Close them again. Afterwards the server must accept connections once + " more, which also verifies the client count is decremented. + for ch in channels + if ch_status(ch) == 'open' + call ch_close(ch) + endif + endfor + call WaitForAssert({-> assert_match('channel:2001', + \ system(actual .. ' --remote-expr "v:servername"'))}) + + call system(actual .. " --remote-expr 'execute(\"qa!\")'") + try + call WaitForAssert({-> assert_equal("dead", job_status(job))}) + finally + if job_status(job) != 'dead' + call assert_report('Server did not exit') + call job_stop(job, 'kill') + endif + endtry +endfunc + " Test if --remote-wait works properly with multiple files func Test_client_server_multiple_remote_wait() CheckRunVimInTerminal diff --git a/src/version.c b/src/version.c index 9afff74b32..a5cca3061b 100644 --- a/src/version.c +++ b/src/version.c @@ -758,6 +758,8 @@ static char *(features[]) = static int included_patches[] = { /* Add new patch number below this line */ +/**/ + 842, /**/ 841, /**/ diff --git a/src/vim.h b/src/vim.h index c04e5f389c..408cf711d1 100644 --- a/src/vim.h +++ b/src/vim.h @@ -2904,6 +2904,15 @@ typedef int (*opt_expand_cb_T)(optexpand_T *args, int *numMatches, char_u ***mat # define MAX_OPEN_CHANNELS 0 #endif +// Maximum number of simultaneously accepted socketserver client channels. +// Bounds the channels (and file descriptors) added to the select()/poll() +// sets so a connection flood cannot overflow the fd_set / pollfd arrays. +#ifdef FEAT_SOCKETSERVER +# define MAX_CLIENT_CHANNELS 100 +#else +# define MAX_CLIENT_CHANNELS 0 +#endif + #if defined(MSWIN) # define MAX_NAMED_PIPE_SIZE 65535 #endif