From 3520e833a448cfae7f52a1b3f4de0707989bd5f5 Mon Sep 17 00:00:00 2001 From: nicm Date: Tue, 26 Aug 2025 07:00:22 +0000 Subject: [PATCH 1/3] Be more robust against misbehaving clients, prompted by deraadt based on a report from sai02 at student dot ubc dot ca. --- server-client.c | 91 ++++++++++++++++++++++++++++--------------------- 1 file changed, 53 insertions(+), 38 deletions(-) diff --git a/server-client.c b/server-client.c index 89486b7c..6a82900a 100644 --- a/server-client.c +++ b/server-client.c @@ -61,9 +61,9 @@ static void server_client_set_path(struct client *); static void server_client_reset_state(struct client *); static void server_client_update_latest(struct client *); static void server_client_dispatch(struct imsg *, void *); -static void server_client_dispatch_command(struct client *, struct imsg *); -static void server_client_dispatch_identify(struct client *, struct imsg *); -static void server_client_dispatch_shell(struct client *); +static int server_client_dispatch_command(struct client *, struct imsg *); +static int server_client_dispatch_identify(struct client *, struct imsg *); +static int server_client_dispatch_shell(struct client *); static void server_client_report_theme(struct client *, enum client_theme); /* Compare client windows. */ @@ -3343,14 +3343,16 @@ server_client_dispatch(struct imsg *imsg, void *arg) case MSG_IDENTIFY_TERMINFO: case MSG_IDENTIFY_TTYNAME: case MSG_IDENTIFY_DONE: - server_client_dispatch_identify(c, imsg); + if (server_client_dispatch_identify(c, imsg) != 0) + goto bad; break; case MSG_COMMAND: - server_client_dispatch_command(c, imsg); + if (server_client_dispatch_command(c, imsg) != 0) + goto bad; break; case MSG_RESIZE: if (datalen != 0) - fatalx("bad MSG_RESIZE size"); + goto bad; if (c->flags & CLIENT_CONTROL) break; @@ -3368,7 +3370,7 @@ server_client_dispatch(struct imsg *imsg, void *arg) break; case MSG_EXITING: if (datalen != 0) - fatalx("bad MSG_EXITING size"); + goto bad; server_client_set_session(c, NULL); recalculate_sizes(); tty_close(&c->tty); @@ -3377,7 +3379,7 @@ server_client_dispatch(struct imsg *imsg, void *arg) case MSG_WAKEUP: case MSG_UNLOCK: if (datalen != 0) - fatalx("bad MSG_WAKEUP size"); + goto bad; if (!(c->flags & CLIENT_SUSPENDED)) break; @@ -3399,9 +3401,9 @@ server_client_dispatch(struct imsg *imsg, void *arg) break; case MSG_SHELL: if (datalen != 0) - fatalx("bad MSG_SHELL size"); - - server_client_dispatch_shell(c); + goto bad; + if (server_client_dispatch_shell(c) != 0) + goto bad; break; case MSG_WRITE_READY: file_write_ready(&c->files, imsg); @@ -3413,6 +3415,12 @@ server_client_dispatch(struct imsg *imsg, void *arg) file_read_done(&c->files, imsg); break; } + + return; + +bad: + log_debug("client %p invalid message type %d", c, imsg->hdr.type); + proc_kill_peer(c->peer); } /* Callback when command is not allowed. */ @@ -3440,7 +3448,7 @@ server_client_command_done(struct cmdq_item *item, __unused void *data) } /* Handle command message. */ -static void +static int server_client_dispatch_command(struct client *c, struct imsg *imsg) { struct msg_command data; @@ -3454,16 +3462,16 @@ server_client_dispatch_command(struct client *c, struct imsg *imsg) struct cmd_list *cmdlist; if (c->flags & CLIENT_EXIT) - return; + return (0); if (imsg->hdr.len - IMSG_HEADER_SIZE < sizeof data) - fatalx("bad MSG_COMMAND size"); + return (-1); memcpy(&data, imsg->data, sizeof data); buf = (char *)imsg->data + sizeof data; - len = imsg->hdr.len - IMSG_HEADER_SIZE - sizeof data; + len = imsg->hdr.len - IMSG_HEADER_SIZE - sizeof data; if (len > 0 && buf[len - 1] != '\0') - fatalx("bad MSG_COMMAND string"); + return (-1); if (cmd_unpack_argv(buf, len, data.argc, &argv) != 0) { cause = xstrdup("command too long"); @@ -3499,7 +3507,7 @@ server_client_dispatch_command(struct client *c, struct imsg *imsg) cmdq_append(c, cmdq_get_callback(server_client_command_done, NULL)); cmd_list_free(cmdlist); - return; + return (0); error: cmd_free_argv(argc, argv); @@ -3508,10 +3516,11 @@ error: free(cause); c->flags |= CLIENT_EXIT; + return (0); } /* Handle identify message. */ -static void +static int server_client_dispatch_identify(struct client *c, struct imsg *imsg) { const char *data, *home; @@ -3521,7 +3530,7 @@ server_client_dispatch_identify(struct client *c, struct imsg *imsg) char *name; if (c->flags & CLIENT_IDENTIFIED) - fatalx("out-of-order identify message"); + return (-1); data = imsg->data; datalen = imsg->hdr.len - IMSG_HEADER_SIZE; @@ -3529,7 +3538,7 @@ server_client_dispatch_identify(struct client *c, struct imsg *imsg) switch (imsg->hdr.type) { case MSG_IDENTIFY_FEATURES: if (datalen != sizeof feat) - fatalx("bad MSG_IDENTIFY_FEATURES size"); + return (-1); memcpy(&feat, data, sizeof feat); c->term_features |= feat; log_debug("client %p IDENTIFY_FEATURES %s", c, @@ -3537,14 +3546,14 @@ server_client_dispatch_identify(struct client *c, struct imsg *imsg) break; case MSG_IDENTIFY_FLAGS: if (datalen != sizeof flags) - fatalx("bad MSG_IDENTIFY_FLAGS size"); + return (-1); memcpy(&flags, data, sizeof flags); c->flags |= flags; log_debug("client %p IDENTIFY_FLAGS %#x", c, flags); break; case MSG_IDENTIFY_LONGFLAGS: if (datalen != sizeof longflags) - fatalx("bad MSG_IDENTIFY_LONGFLAGS size"); + return (-1); memcpy(&longflags, data, sizeof longflags); c->flags |= longflags; log_debug("client %p IDENTIFY_LONGFLAGS %#llx", c, @@ -3552,16 +3561,13 @@ server_client_dispatch_identify(struct client *c, struct imsg *imsg) break; case MSG_IDENTIFY_TERM: if (datalen == 0 || data[datalen - 1] != '\0') - fatalx("bad MSG_IDENTIFY_TERM string"); - if (*data == '\0') - c->term_name = xstrdup("unknown"); - else - c->term_name = xstrdup(data); + return (-1); + c->term_name = xstrdup(data); log_debug("client %p IDENTIFY_TERM %s", c, data); break; case MSG_IDENTIFY_TERMINFO: if (datalen == 0 || data[datalen - 1] != '\0') - fatalx("bad MSG_IDENTIFY_TERMINFO string"); + return (-1); c->term_caps = xreallocarray(c->term_caps, c->term_ncaps + 1, sizeof *c->term_caps); c->term_caps[c->term_ncaps++] = xstrdup(data); @@ -3569,13 +3575,13 @@ server_client_dispatch_identify(struct client *c, struct imsg *imsg) break; case MSG_IDENTIFY_TTYNAME: if (datalen == 0 || data[datalen - 1] != '\0') - fatalx("bad MSG_IDENTIFY_TTYNAME string"); + return (-1); c->ttyname = xstrdup(data); log_debug("client %p IDENTIFY_TTYNAME %s", c, data); break; case MSG_IDENTIFY_CWD: if (datalen == 0 || data[datalen - 1] != '\0') - fatalx("bad MSG_IDENTIFY_CWD string"); + return (-1); if (access(data, X_OK) == 0) c->cwd = xstrdup(data); else if ((home = find_home()) != NULL) @@ -3586,26 +3592,26 @@ server_client_dispatch_identify(struct client *c, struct imsg *imsg) break; case MSG_IDENTIFY_STDIN: if (datalen != 0) - fatalx("bad MSG_IDENTIFY_STDIN size"); + return (-1); c->fd = imsg_get_fd(imsg); log_debug("client %p IDENTIFY_STDIN %d", c, c->fd); break; case MSG_IDENTIFY_STDOUT: if (datalen != 0) - fatalx("bad MSG_IDENTIFY_STDOUT size"); + return (-1); c->out_fd = imsg_get_fd(imsg); log_debug("client %p IDENTIFY_STDOUT %d", c, c->out_fd); break; case MSG_IDENTIFY_ENVIRON: if (datalen == 0 || data[datalen - 1] != '\0') - fatalx("bad MSG_IDENTIFY_ENVIRON string"); + return (-1); if (strchr(data, '=') != NULL) environ_put(c->environ, data, 0); log_debug("client %p IDENTIFY_ENVIRON %s", c, data); break; case MSG_IDENTIFY_CLIENTPID: if (datalen != sizeof c->pid) - fatalx("bad MSG_IDENTIFY_CLIENTPID size"); + return (-1); memcpy(&c->pid, data, sizeof c->pid); log_debug("client %p IDENTIFY_CLIENTPID %ld", c, (long)c->pid); break; @@ -3614,10 +3620,15 @@ server_client_dispatch_identify(struct client *c, struct imsg *imsg) } if (imsg->hdr.type != MSG_IDENTIFY_DONE) - return; + return (0); c->flags |= CLIENT_IDENTIFIED; - if (*c->ttyname != '\0') + if (c->term_name == NULL || *c->term_name == '\0') { + free(c->term_name); + c->term_name = xstrdup("unknown"); + } + + if (c->ttyname == NULL || *c->ttyname != '\0') name = xstrdup(c->ttyname); else xasprintf(&name, "client-%ld", (long)c->pid); @@ -3634,7 +3645,8 @@ server_client_dispatch_identify(struct client *c, struct imsg *imsg) tty_resize(&c->tty); c->flags |= CLIENT_TERMINAL; } - close(c->out_fd); + if (c->out_fd != -1) + close(c->out_fd); c->out_fd = -1; } @@ -3647,10 +3659,12 @@ server_client_dispatch_identify(struct client *c, struct imsg *imsg) !cfg_finished && c == TAILQ_FIRST(&clients)) start_cfg(); + + return (0); } /* Handle shell message. */ -static void +static int server_client_dispatch_shell(struct client *c) { const char *shell; @@ -3661,6 +3675,7 @@ server_client_dispatch_shell(struct client *c) proc_send(c->peer, MSG_SHELL, -1, shell, strlen(shell) + 1); proc_kill_peer(c->peer); + return (0); } /* Get client working directory. */ From ca0f0419e668b78ccd19d298df5ccf83207e0595 Mon Sep 17 00:00:00 2001 From: nicm Date: Tue, 26 Aug 2025 07:15:00 +0000 Subject: [PATCH 2/3] input_stop_utf8 can move the cursor, so it is now wrong to store the cursor position before calling it. Problem found by Jayakrishna Vadayath. --- input.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/input.c b/input.c index 14aea52a..b513d2c6 100644 --- a/input.c +++ b/input.c @@ -1234,7 +1234,7 @@ input_c0_dispatch(struct input_ctx *ictx) struct window_pane *wp = ictx->wp; struct screen *s = sctx->s; struct grid_cell gc, first_gc; - u_int cx = s->cx, line = s->cy + s->grid->hsize; + u_int cx, line; u_int width; int has_content = 0; @@ -1254,11 +1254,13 @@ input_c0_dispatch(struct input_ctx *ictx) break; case '\011': /* HT */ /* Don't tab beyond the end of the line. */ - if (s->cx >= screen_size_x(s) - 1) + cx = s->cx; + if (cx >= screen_size_x(s) - 1) break; /* Find the next tab point, or use the last column if none. */ - grid_get_cell(s->grid, s->cx, line, &first_gc); + line = s->cy + s->grid->hsize; + grid_get_cell(s->grid, cx, line, &first_gc); do { if (!has_content) { grid_get_cell(s->grid, cx, line, &gc); @@ -2664,7 +2666,7 @@ input_osc_8(struct input_ctx *ictx, const char *p) struct hyperlinks *hl = ictx->ctx.s->hyperlinks; struct grid_cell *gc = &ictx->cell.cell; const char *start, *end, *uri; - char *id = NULL; + char *id = NULL; for (start = p; (end = strpbrk(start, ":;")) != NULL; start = end + 1) { if (end - start >= 4 && strncmp(start, "id=", 3) == 0) { @@ -2859,8 +2861,8 @@ input_osc_52(struct input_ctx *ictx, const char *p) int outlen, state; struct screen_write_ctx ctx; struct paste_buffer *pb; - const char* allow = "cpqs01234567"; - char flags[sizeof "cpqs01234567"] = ""; + const char* allow = "cpqs01234567"; + char flags[sizeof "cpqs01234567"] = ""; u_int i, j = 0; if (wp == NULL) From 0b7235c5f0188da5e84abfe4ccd2219cae4a1d7b Mon Sep 17 00:00:00 2001 From: nicm Date: Tue, 26 Aug 2025 07:17:51 +0000 Subject: [PATCH 3/3] Correctly calculate lines to clear for deletions, from Pavel Roskin. --- grid-view.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/grid-view.c b/grid-view.c index 4d687339..7590ea42 100644 --- a/grid-view.c +++ b/grid-view.c @@ -173,7 +173,7 @@ grid_view_delete_lines(struct grid *gd, u_int py, u_int ny, u_int bg) sy = grid_view_y(gd, gd->sy); grid_move_lines(gd, py, py + ny, sy - py - ny, bg); - grid_clear(gd, 0, sy - ny, gd->sx, py + ny - (sy - ny), bg); + grid_clear(gd, 0, sy - ny, gd->sx, ny, bg); } /* Delete lines inside scroll region. */ @@ -221,7 +221,7 @@ grid_view_delete_cells(struct grid *gd, u_int px, u_int py, u_int nx, u_int bg) sx = grid_view_x(gd, gd->sx); grid_move_cells(gd, px, px + nx, py, sx - px - nx, bg); - grid_clear(gd, sx - nx, py, px + nx - (sx - nx), 1, bg); + grid_clear(gd, sx - nx, py, nx, 1, bg); } /* Convert cells into a string. */