From 81222dac578589d8ee37408ad892250fdd2d6de3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sat, 22 Aug 2020 08:29:35 +0200 Subject: [PATCH 1/7] selection: add a 2 second timeout when receiving clipboard data When reading clipboard data, a malicious clipboard provider could stall us forever, by not sending any data, and not closing the pipe. This commit adds a timer_fd based timeout of 2 seconds. If the timer triggers, we abort the clipboard receive. --- selection.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 78 insertions(+), 11 deletions(-) diff --git a/selection.c b/selection.c index e34c971d..fc7b0308 100644 --- a/selection.c +++ b/selection.c @@ -8,6 +8,7 @@ #include #include +#include #define LOG_MODULE "selection" #define LOG_ENABLE_DBG 0 @@ -993,12 +994,50 @@ selection_to_clipboard(struct seat *seat, struct terminal *term, uint32_t serial } struct clipboard_receive { + int read_fd; + int timeout_fd; + struct itimerspec timeout; + /* Callback data */ void (*cb)(const char *data, size_t size, void *user); void (*done)(void *user); void *user; }; +static void +clipboard_receive_done(struct fdm *fdm, struct clipboard_receive *ctx) +{ + fdm_del(fdm, ctx->timeout_fd); + fdm_del(fdm, ctx->read_fd); + ctx->done(ctx->user); + free(ctx); +} + +static bool +fdm_receive_timeout(struct fdm *fdm, int fd, int events, void *data) +{ + struct clipboard_receive *ctx = data; + if (events & EPOLLHUP) + return false; + + assert(events & EPOLLIN); + + uint64_t expire_count; + ssize_t ret = read(fd, &expire_count, sizeof(expire_count)); + if (ret < 0) { + if (errno == EAGAIN) + return true; + + LOG_ERRNO("failed to read clipboard timeout timer"); + return false; + } + + LOG_WARN("no data received from clipboard in %llu seconds, aborting", + (unsigned long long)ctx->timeout.it_value.tv_sec); + + clipboard_receive_done(fdm, ctx); + return true; +} static bool fdm_receive(struct fdm *fdm, int fd, int events, void *data) @@ -1008,6 +1047,12 @@ fdm_receive(struct fdm *fdm, int fd, int events, void *data) if ((events & EPOLLHUP) && !(events & EPOLLIN)) goto done; + /* Reset timeout timer */ + if (timerfd_settime(ctx->timeout_fd, 0, &ctx->timeout, NULL) < 0) { + LOG_ERRNO("failed to re-arm clipboard timeout timer"); + return false; + } + /* Read until EOF */ while (true) { char text[256]; @@ -1044,9 +1089,7 @@ fdm_receive(struct fdm *fdm, int fd, int events, void *data) } done: - fdm_del(fdm, fd); - ctx->done(ctx->user); - free(ctx); + clipboard_receive_done(fdm, ctx); return true; } @@ -1055,28 +1098,52 @@ begin_receive_clipboard(struct terminal *term, int read_fd, void (*cb)(const char *data, size_t size, void *user), void (*done)(void *user), void *user) { + int timeout_fd = -1; + struct clipboard_receive *ctx = NULL; + int flags; if ((flags = fcntl(read_fd, F_GETFL)) < 0 || fcntl(read_fd, F_SETFL, flags | O_NONBLOCK) < 0) { LOG_ERRNO("failed to set O_NONBLOCK"); - close(read_fd); - done(user); - return; + goto err; } - struct clipboard_receive *ctx = xmalloc(sizeof(*ctx)); + timeout_fd = timerfd_create(CLOCK_MONOTONIC, TFD_NONBLOCK | TFD_CLOEXEC); + if (timeout_fd < 0) { + LOG_ERRNO("failed to create clipboard timeout timer FD"); + goto err; + } + + const struct itimerspec timeout = {.it_value = {.tv_sec = 2}}; + if (timerfd_settime(timeout_fd, 0, &timeout, NULL) < 0) { + LOG_ERRNO("faild to arm clipboard timeout timer"); + goto err; + } + + ctx = xmalloc(sizeof(*ctx)); *ctx = (struct clipboard_receive) { + .read_fd = read_fd, + .timeout_fd = timeout_fd, + .timeout = timeout, .cb = cb, .done = done, .user = user, }; - if (!fdm_add(term->fdm, read_fd, EPOLLIN, &fdm_receive, ctx)) { - close(read_fd); - free(ctx); - done(user); + if (!fdm_add(term->fdm, read_fd, EPOLLIN, &fdm_receive, ctx) || + !fdm_add(term->fdm, timeout_fd, EPOLLIN, &fdm_receive_timeout, ctx)) + { + goto err; } + + return; + +err: + free(ctx); + fdm_del(term->fdm, timeout_fd); + fdm_del(term->fdm, read_fd); + done(user); } void From e570146c07b43ffacde6518e35b734b87095ed56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sat, 22 Aug 2020 09:14:18 +0200 Subject: [PATCH 2/7] selection: block non-paste data from being sent to client while pasting While pasting data from the clipboard, block *all* other data from being sent to the client. This includes keyboard and mouse events, but also replies for VT queries. This is particularly important when bracketed paste has been enabled, since then the client will interpret *everything* between the bracketed paste start and end as paste data. --- selection.c | 15 ++++- terminal.c | 163 ++++++++++++++++++++++++++++++++++++---------------- terminal.h | 8 ++- 3 files changed, 133 insertions(+), 53 deletions(-) diff --git a/selection.c b/selection.c index fc7b0308..4788ce61 100644 --- a/selection.c +++ b/selection.c @@ -1182,7 +1182,7 @@ static void from_clipboard_cb(const char *data, size_t size, void *user) { struct terminal *term = user; - term_to_slave(term, data, size); + term_paste_data_to_slave(term, data, size); } static void @@ -1191,7 +1191,13 @@ from_clipboard_done(void *user) struct terminal *term = user; if (term->bracketed_paste) - term_to_slave(term, "\033[201~", 6); + term_paste_data_to_slave(term, "\033[201~", 6); + + term->is_sending_paste_data = false; + + /* Make sure we send any queued up non-paste data */ + if (tll_length(term->ptmx_buffers) > 0) + fdm_event_add(term->fdm, term->ptmx, EPOLLOUT); } void @@ -1201,6 +1207,8 @@ selection_from_clipboard(struct seat *seat, struct terminal *term, uint32_t seri if (clipboard->data_offer == NULL) return; + term->is_sending_paste_data = true; + if (term->bracketed_paste) term_to_slave(term, "\033[200~", 6); @@ -1312,8 +1320,9 @@ selection_from_primary(struct seat *seat, struct terminal *term) if (clipboard->data_offer == NULL) return; + term->is_sending_paste_data = true; if (term->bracketed_paste) - term_to_slave(term, "\033[200~", 6); + term_paste_data_to_slave(term, "\033[200~", 6); text_from_primary(seat, term, &from_clipboard_cb, &from_clipboard_done, term); } diff --git a/terminal.c b/terminal.c index c852952a..08ef11c2 100644 --- a/terminal.c +++ b/terminal.c @@ -50,31 +50,44 @@ const char *const XCURSOR_RIGHT_SIDE = "right_side"; const char *const XCURSOR_TOP_SIDE = "top_side"; const char *const XCURSOR_BOTTOM_SIDE = "bottom_side"; -bool -term_to_slave(struct terminal *term, const void *_data, size_t len) +static void +enqueue_data_for_slave(const void *data, size_t len, size_t offset, + ptmx_buffer_list_t *buffer_list) { - if (term->ptmx < 0) { - /* We're probably in "hold" */ - return false; - } + /* + * We're in asynchronous mode - push data to queue and let the FDM + * handler take care of it + */ + { + void *copy = xmalloc(len); + memcpy(copy, data, len); - size_t async_idx = 0; - if (tll_length(term->ptmx_buffer) > 0) { - /* With a non-empty queue, EPOLLOUT has already been enabled */ - goto enqueue_data; + struct ptmx_buffer queued = { + .data = copy, + .len = len, + .idx = offset, + }; + tll_push_back(*buffer_list, queued); } +} +static bool +data_to_slave(struct terminal *term, const void *data, size_t len, + ptmx_buffer_list_t *buffer_list) +{ /* * Try a synchronous write first. If we fail to write everything, * switch to asynchronous. */ - switch (async_write(term->ptmx, _data, len, &async_idx)) { + size_t async_idx = 0; + switch (async_write(term->ptmx, data, len, &async_idx)) { case ASYNC_WRITE_REMAIN: /* Switch to asynchronous mode; let FDM write the remaining data */ if (!fdm_event_add(term->fdm, term->ptmx, EPOLLOUT)) return false; - goto enqueue_data; + enqueue_data_for_slave(data, len, async_idx, buffer_list); + return true; case ASYNC_WRITE_DONE: return true; @@ -87,24 +100,52 @@ term_to_slave(struct terminal *term, const void *_data, size_t len) /* Shouldn't get here */ assert(false); return false; +} -enqueue_data: - /* - * We're in asynchronous mode - push data to queue and let the FDM - * handler take care of it - */ - { - void *copy = xmalloc(len); - memcpy(copy, _data, len); +bool +term_paste_data_to_slave(struct terminal *term, const void *data, size_t len) +{ + assert(term->is_sending_paste_data); - struct ptmx_buffer queued = { - .data = copy, - .len = len, - .idx = async_idx, - }; - tll_push_back(term->ptmx_buffer, queued); + if (term->ptmx < 0) { + /* We're probably in "hold" */ + return false; } - return true; + + if (tll_length(term->ptmx_paste_buffers) > 0) { + /* Don't even try to send data *now* if there's queued up + * data, since that would result in events arriving out of + * order. */ + enqueue_data_for_slave(data, len, 0, &term->ptmx_paste_buffers); + return true; + } + + return data_to_slave(term, data, len, &term->ptmx_paste_buffers); +} + +bool +term_to_slave(struct terminal *term, const void *data, size_t len) +{ + if (term->ptmx < 0) { + /* We're probably in "hold" */ + return false; + } + + if (tll_length(term->ptmx_buffers) > 0 || term->is_sending_paste_data) { + /* + * Don't even try to send data *now* if there's queued up + * data, since that would result in events arriving out of + * order. + * + * Furthermore, if we're currently sending paste data to the + * client, do *not* mix that stream with other events + * (https://codeberg.org/dnkl/foot/issues/101). + */ + enqueue_data_for_slave(data, len, 0, &term->ptmx_buffers); + return true; + } + + return data_to_slave(term, data, len, &term->ptmx_buffers); } static bool @@ -113,28 +154,48 @@ fdm_ptmx_out(struct fdm *fdm, int fd, int events, void *data) struct terminal *term = data; /* If there is no queued data, then we shouldn't be in asynchronous mode */ - assert(tll_length(term->ptmx_buffer) > 0); + assert(tll_length(term->ptmx_buffers) > 0 || + tll_length(term->ptmx_paste_buffers) > 0); - /* Don't use pop() since we may not be able to write the entire buffer */ - tll_foreach(term->ptmx_buffer, it) { - switch (async_write(term->ptmx, it->item.data, it->item.len, &it->item.idx)) { - case ASYNC_WRITE_DONE: - free(it->item.data); - tll_remove(term->ptmx_buffer, it); - break; - - case ASYNC_WRITE_REMAIN: - /* to_slave() updated it->item.idx */ - return true; - - case ASYNC_WRITE_ERR: - LOG_ERRNO("failed to asynchronously write %zu bytes to slave", - it->item.len - it->item.idx); - return false; - } + /* Writes a single buffer, returns if not all of it could be written */ +#define write_one_buffer(buffer_list) \ + { \ + switch (async_write(term->ptmx, it->item.data, it->item.len, &it->item.idx)) { \ + case ASYNC_WRITE_DONE: \ + free(it->item.data); \ + tll_remove(buffer_list, it); \ + break; \ + case ASYNC_WRITE_REMAIN: \ + /* to_slave() updated it->item.idx */ \ + return true; \ + case ASYNC_WRITE_ERR: \ + LOG_ERRNO("failed to asynchronously write %zu bytes to slave", \ + it->item.len - it->item.idx); \ + return false; \ + } \ } - /* No more queued data, switch back to synchronous mode */ + tll_foreach(term->ptmx_paste_buffers, it) + write_one_buffer(term->ptmx_paste_buffers); + + /* If we get here, *all* paste data buffers were successfully + * flushed */ + + if (!term->is_sending_paste_data) { + tll_foreach(term->ptmx_buffers, it) + write_one_buffer(term->ptmx_buffers); + } + + /* + * If we get here, *all* buffers were successfully flushed. + * + * Or, we're still sending paste data, in which case we do *not* + * want to send the "normal" queued up data + * + * In both cases, we want to *disable* the FDM callback since + * otherwise we'd just be called right away again, with nothing to + * write. + */ fdm_event_del(term->fdm, term->ptmx, EPOLLOUT); return true; } @@ -840,7 +901,8 @@ term_init(const struct config *conf, struct fdm *fdm, struct reaper *reaper, .conf = conf, .quit = false, .ptmx = ptmx, - .ptmx_buffer = tll_init(), + .ptmx_buffers = tll_init(), + .ptmx_paste_buffers = tll_init(), .font_sizes = xmalloc(sizeof(term->font_sizes[0]) * tll_length(conf->fonts)), .font_dpi = 0., .font_subpixel = (conf->colors.alpha == 0xffff /* Can't do subpixel rendering on transparent background */ @@ -1206,9 +1268,12 @@ term_destroy(struct terminal *term) assert(tll_length(term->render.workers.queue) == 0); tll_free(term->render.workers.queue); - tll_foreach(term->ptmx_buffer, it) + tll_foreach(term->ptmx_buffers, it) free(it->item.data); - tll_free(term->ptmx_buffer); + tll_free(term->ptmx_buffers); + tll_foreach(term->ptmx_paste_buffers, it) + free(it->item.data); + tll_free(term->ptmx_paste_buffers); tll_free(term->tab_stops); tll_foreach(term->normal.sixel_images, it) diff --git a/terminal.h b/terminal.h index d1235c2f..4318f889 100644 --- a/terminal.h +++ b/terminal.h @@ -205,6 +205,8 @@ enum term_surface { TERM_SURF_BUTTON_CLOSE, }; +typedef tll(struct ptmx_buffer) ptmx_buffer_list_t; + struct terminal { struct fdm *fdm; struct reaper *reaper; @@ -226,7 +228,9 @@ struct terminal { float font_dpi; enum fcft_subpixel font_subpixel; - tll(struct ptmx_buffer) ptmx_buffer; + bool is_sending_paste_data; + ptmx_buffer_list_t ptmx_buffers; + ptmx_buffer_list_t ptmx_paste_buffers; enum cursor_origin origin; enum cursor_keys cursor_keys_mode; @@ -489,6 +493,8 @@ int term_destroy(struct terminal *term); void term_reset(struct terminal *term, bool hard); bool term_to_slave(struct terminal *term, const void *data, size_t len); +bool term_paste_data_to_slave( + struct terminal *term, const void *data, size_t len); bool term_font_size_increase(struct terminal *term); bool term_font_size_decrease(struct terminal *term); From 2762e044aa293fe3fe7b9e73f618d5b4c0add400 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sat, 22 Aug 2020 09:16:51 +0200 Subject: [PATCH 3/7] changelog: fixed input events from getting mixed with paste data --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 76fa3802..2b5f4052 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -86,6 +86,8 @@ (https://codeberg.org/dnkl/foot/issues/97). * Mouse events from being sent to client application when a mouse binding has consumed it. +* Input events from getting mixed with paste data + (https://codeberg.org/dnkl/foot/issues/101). ### Security From efbc3431ed5f89f7a050af35d54ceb76e77e9aa8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sat, 22 Aug 2020 09:23:58 +0200 Subject: [PATCH 4/7] selection: clipboard callback: assert we're in sending-paste-data mode --- selection.c | 1 + 1 file changed, 1 insertion(+) diff --git a/selection.c b/selection.c index 4788ce61..800f7451 100644 --- a/selection.c +++ b/selection.c @@ -1182,6 +1182,7 @@ static void from_clipboard_cb(const char *data, size_t size, void *user) { struct terminal *term = user; + assert(term->is_sending_paste_data); term_paste_data_to_slave(term, data, size); } From 91a839bf8ee146e6c43152f701431a77fa34b940 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sat, 22 Aug 2020 09:25:25 +0200 Subject: [PATCH 5/7] term: enqueue data: cleanup --- terminal.c | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/terminal.c b/terminal.c index 08ef11c2..9d68167d 100644 --- a/terminal.c +++ b/terminal.c @@ -54,21 +54,15 @@ static void enqueue_data_for_slave(const void *data, size_t len, size_t offset, ptmx_buffer_list_t *buffer_list) { - /* - * We're in asynchronous mode - push data to queue and let the FDM - * handler take care of it - */ - { - void *copy = xmalloc(len); - memcpy(copy, data, len); + void *copy = xmalloc(len); + memcpy(copy, data, len); - struct ptmx_buffer queued = { - .data = copy, - .len = len, - .idx = offset, - }; - tll_push_back(*buffer_list, queued); - } + struct ptmx_buffer queued = { + .data = copy, + .len = len, + .idx = offset, + }; + tll_push_back(*buffer_list, queued); } static bool From 9ec4c3fd947d8160ce38b9892e3172440bffe667 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sat, 22 Aug 2020 16:20:49 +0200 Subject: [PATCH 6/7] selection: must use term_paste_data_to_slave() for paste data --- selection.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/selection.c b/selection.c index 800f7451..f86814a1 100644 --- a/selection.c +++ b/selection.c @@ -1211,7 +1211,7 @@ selection_from_clipboard(struct seat *seat, struct terminal *term, uint32_t seri term->is_sending_paste_data = true; if (term->bracketed_paste) - term_to_slave(term, "\033[200~", 6); + term_paste_data_to_slave(term, "\033[200~", 6); text_from_clipboard( seat, term, &from_clipboard_cb, &from_clipboard_done, term); From 1707db167815348eff3a3e8e7c5a73130df678ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Sat, 22 Aug 2020 16:30:52 +0200 Subject: [PATCH 7/7] selection: don't initiate a paste when we're already pasting --- selection.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/selection.c b/selection.c index f86814a1..9428adff 100644 --- a/selection.c +++ b/selection.c @@ -1204,6 +1204,11 @@ from_clipboard_done(void *user) void selection_from_clipboard(struct seat *seat, struct terminal *term, uint32_t serial) { + if (term->is_sending_paste_data) { + /* We're already pasting... */ + return; + } + struct wl_clipboard *clipboard = &seat->clipboard; if (clipboard->data_offer == NULL) return; @@ -1317,6 +1322,11 @@ selection_from_primary(struct seat *seat, struct terminal *term) if (term->wl->primary_selection_device_manager == NULL) return; + if (term->is_sending_paste_data) { + /* We're already pasting... */ + return; + } + struct wl_clipboard *clipboard = &seat->clipboard; if (clipboard->data_offer == NULL) return;