From e2197b838992c3880dec09a185a436a34161cfa7 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Fri, 17 Apr 2026 15:38:39 +1000 Subject: [PATCH 01/20] util: xrealloc should take a size_t For correctness only because this is C and types are mostly just there for lolz, not because they're actualy useful. Assisted-by: Claude:claude-opus-4-6 Part-of: --- src/util-mem.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util-mem.h b/src/util-mem.h index c02affd..bacc708 100644 --- a/src/util-mem.h +++ b/src/util-mem.h @@ -117,7 +117,7 @@ xalloc(size_t size) } static inline void * -xrealloc(void *ptr, int size) +xrealloc(void *ptr, size_t size) { void *tmp = realloc(ptr, size); assert(tmp); From bbcc808b1a4064fcd3e4a41eb08fb3e7e621bbca Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Fri, 17 Apr 2026 15:41:47 +1000 Subject: [PATCH 02/20] util: use x{re}alloc, not the libc ones Might as well blow up when allocation fails, not that this ever happens. And in the case of cmdline_as_str() this now also means we definitely have to free the result (we did before but well, there was the theoretical case of it failling). Assisted-by: Claude:claude-opus-4-6 Part-of: --- src/util-io.c | 10 +++------- src/util-strings.c | 8 ++------ src/util-strings.h | 4 ++-- 3 files changed, 7 insertions(+), 15 deletions(-) diff --git a/src/util-io.c b/src/util-io.c index 7b39b6c..f3f7af8 100644 --- a/src/util-io.c +++ b/src/util-io.c @@ -126,11 +126,8 @@ struct iobuf { struct iobuf * iobuf_new(size_t size) { - struct iobuf *buf = malloc(sizeof(*buf)); - uint8_t *data = malloc(size); - - assert(buf); - assert(data); + struct iobuf *buf = xalloc(sizeof(*buf)); + uint8_t *data = xalloc(size); *buf = (struct iobuf){ .sz = size, @@ -212,8 +209,7 @@ iobuf_take_fd(struct iobuf *buf) static inline void iobuf_resize(struct iobuf *buf, size_t to_size) { - uint8_t *newdata = realloc(buf->data, to_size); - assert(newdata); + uint8_t *newdata = xrealloc(buf->data, to_size); buf->data = newdata; buf->sz = to_size; diff --git a/src/util-strings.c b/src/util-strings.c index 656426d..71a3387 100644 --- a/src/util-strings.c +++ b/src/util-strings.c @@ -214,9 +214,7 @@ strreplace(const char *string, const char *separator, const char *replacement) destptr += len; } - void *tmp = realloc(r, (destptr - r) + 1); - assert(tmp); - return tmp; + return xrealloc(r, (destptr - r) + 1); } size_t @@ -240,9 +238,7 @@ strv_append_take(char **strv, char **str) size_t len = strv_len(strv) + 1; len = max(len, 2); - char **s = realloc(strv, len * sizeof(*strv)); - if (!s) - abort(); + char **s = xrealloc(strv, len * sizeof(*strv)); s[len - 1] = NULL; s[len - 2] = *str; *str = NULL; diff --git a/src/util-strings.h b/src/util-strings.h index 74b7f38..f82874d 100644 --- a/src/util-strings.h +++ b/src/util-strings.h @@ -441,11 +441,11 @@ cmdline_as_str(void) if (sysctl(mib, ARRAY_LENGTH(mib), NULL, &len, NULL, 0)) return NULL; - char *const procargs = malloc(len); + _cleanup_free_ char *procargs = xalloc(len); if (sysctl(mib, ARRAY_LENGTH(mib), procargs, &len, NULL, 0)) return NULL; - return procargs; + return steal(&procargs); #else int fd = open("/proc/self/cmdline", O_RDONLY); if (fd != -1) { From bacd62cae0043e2c4f98cc5e728d8f458b20e8d8 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Mon, 11 May 2026 14:00:51 +1000 Subject: [PATCH 03/20] util: fix xatou_base for values > UINT_MAX The check `(long)v < 0` only catches values >= LONG_MAX when cast to signed long. On 64-bit systems where sizeof(long) == 8, values between UINT_MAX+1 and LONG_MAX pass this check but silently truncate when assigned to the unsigned int output parameter. Use `v > UINT_MAX` for a correct range check. Assisted-by: Claude:claude-opus-4-6 Part-of: --- src/util-strings.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++ src/util-strings.h | 2 +- 2 files changed, 67 insertions(+), 1 deletion(-) diff --git a/src/util-strings.c b/src/util-strings.c index 71a3387..41c1d95 100644 --- a/src/util-strings.c +++ b/src/util-strings.c @@ -658,4 +658,70 @@ MUNIT_TEST(test_strv_from_mem) return MUNIT_OK; } + +MUNIT_TEST(test_xatou) +{ + unsigned int val; + + munit_assert_true(xatou("0", &val)); + munit_assert_uint(val, ==, 0); + + munit_assert_true(xatou("1", &val)); + munit_assert_uint(val, ==, 1); + + munit_assert_true(xatou("123", &val)); + munit_assert_uint(val, ==, 123); + + /* UINT_MAX is the upper boundary, must succeed */ + munit_assert_true(xatou("4294967295", &val)); + munit_assert_uint(val, ==, UINT_MAX); + + /* UINT_MAX + 1 must fail */ + munit_assert_false(xatou("4294967296", &val)); + + /* Another random value in the range UINT_MAX < val < LONG_MAX */ + munit_assert_false(xatou("8589934592", &val)); + + /* LONG_MAX as string - must fail */ + munit_assert_false(xatou("9223372036854775807", &val)); + + /* Overflow beyond ULONG_MAX - strtoul sets errno, must fail */ + munit_assert_false(xatou("18446744073709551616", &val)); + + /* negative numbers: strtoul wraps "-1" to ULONG_MAX without + * setting errno, but v > UINT_MAX catches it */ + munit_assert_false(xatou("-1", &val)); + + /* invalid strings */ + munit_assert_false(xatou("", &val)); + munit_assert_false(xatou("abc", &val)); + munit_assert_false(xatou("123abc", &val)); + munit_assert_false(xatou("12 34", &val)); + + /* hex via xatou_base */ + munit_assert_true(xatou_base("ff", &val, 16)); + munit_assert_uint(val, ==, 0xff); + + munit_assert_true(xatou_base("0xff", &val, 16)); + munit_assert_uint(val, ==, 0xff); + + munit_assert_true(xatou_base("FFFFFFFF", &val, 16)); + munit_assert_uint(val, ==, UINT_MAX); + + /* hex UINT_MAX + 1 */ + munit_assert_false(xatou_base("100000000", &val, 16)); + + /* octal */ + munit_assert_true(xatou_base("77", &val, 8)); + munit_assert_uint(val, ==, 077); + + munit_assert_true(xatou_base("37777777777", &val, 8)); + munit_assert_uint(val, ==, UINT_MAX); + + /* octal UINT_MAX + 1 */ + munit_assert_false(xatou_base("40000000000", &val, 8)); + + return MUNIT_OK; +} + #endif diff --git a/src/util-strings.h b/src/util-strings.h index f82874d..e62df4c 100644 --- a/src/util-strings.h +++ b/src/util-strings.h @@ -183,7 +183,7 @@ xatou_base(const char *str, unsigned int *val, int base) if (*str != '\0' && *endptr != '\0') return false; - if ((long)v < 0) + if (v > UINT_MAX) return false; *val = v; From 4440f15453d752d753ef335c09a24d0715275efd Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Fri, 17 Apr 2026 18:50:23 +1000 Subject: [PATCH 04/20] util: return NULL in memmap_new() for a size 0 Assisted-by: Claude:claude-opus-4-6 Part-of: --- src/util-memmap.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/util-memmap.c b/src/util-memmap.c index a106ef2..31445f7 100644 --- a/src/util-memmap.c +++ b/src/util-memmap.c @@ -55,6 +55,9 @@ OBJECT_IMPLEMENT_GETTER(memmap, data, void *); struct memmap * memmap_new(int fd, size_t sz) { + if (sz == 0) + return NULL; + _unref_(memmap) *memmap = memmap_create(NULL); void *map = mmap(NULL, sz, PROT_READ, MAP_PRIVATE, fd, 0); From c6c8f47b432616b1469c6fc7b03246f3f3c08fa2 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Mon, 11 May 2026 11:37:14 +1000 Subject: [PATCH 05/20] util: add a steal_fd() helper function We can't use steal for fds because of the -1 requirement. Assisted-by: Claude:claude-opus-4-6 Part-of: --- src/util-mem.h | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/util-mem.h b/src/util-mem.h index bacc708..7271f6f 100644 --- a/src/util-mem.h +++ b/src/util-mem.h @@ -96,6 +96,21 @@ _steal(void *ptr) #define steal(ptr_) \ (typeof(*ptr_))_steal(ptr_) +/** + * Resets the pointer content to -1 and returns + * the original value. + */ +static inline int +steal_fd(int *ptr) +{ + if (ptr) { + int original = *ptr; + *ptr = -1; + return original; + } + return -1; +} + /** * Never-failing calloc with a size limit check. */ From 1efc6d03d13ee444a7b10abb4a4a35309d811cd8 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Mon, 11 May 2026 11:40:46 +1000 Subject: [PATCH 06/20] util: only close fds larger than -1 in cleanup_close This allows us to pass xerrno() results (i.e. negative errnos) and still do the right thing. Assisted-by: Claude:claude-opus-4-6 Part-of: --- src/util-io.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util-io.h b/src/util-io.h index 26cf02a..21d5d81 100644 --- a/src/util-io.h +++ b/src/util-io.h @@ -127,7 +127,7 @@ xerrno(int value) static inline int xclose(int fd) { - if (fd != -1) { + if (fd > -1) { /* Not SYSCALL(), see libei MR!261#note_2131802 */ close(fd); } From f12fe4ce14b07131a43f56d7edabe5b3fc829841 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Mon, 11 May 2026 11:42:12 +1000 Subject: [PATCH 07/20] util: fix possible fd leak if xconnect fails on connect() Assisted-by: Claude:claude-opus-4-6 Part-of: --- src/util-io.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/util-io.h b/src/util-io.h index 21d5d81..0ea7d63 100644 --- a/src/util-io.h +++ b/src/util-io.h @@ -223,7 +223,8 @@ xconnect(const char *path) if (!xsnprintf(addr.sun_path, sizeof(addr.sun_path), "%s", path)) return -EINVAL; - int sockfd = xerrno(SYSCALL(socket(AF_UNIX, SOCK_STREAM | SOCK_NONBLOCK, 0))); + _cleanup_close_ int sockfd = + xerrno(SYSCALL(socket(AF_UNIX, SOCK_STREAM | SOCK_NONBLOCK, 0))); if (sockfd < 0) return sockfd; @@ -231,7 +232,7 @@ xconnect(const char *path) if (rc < 0) return rc; - return sockfd; + return steal_fd(&sockfd); } /** From 1b4a0d7c1a7b11986cc9f1e26284d7caefe5b353 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Mon, 11 May 2026 11:46:40 +1000 Subject: [PATCH 08/20] util: handle failing memfd sealing Assisted-by: Claude:claude-opus-4-6 Part-of: --- src/util-memfile.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/util-memfile.c b/src/util-memfile.c index ea55c46..6f5e334 100644 --- a/src/util-memfile.c +++ b/src/util-memfile.c @@ -63,7 +63,8 @@ memfile_new(const char *data, size_t sz) if (fd < 0) return NULL; - fcntl(fd, F_ADD_SEALS, F_SEAL_SHRINK); + if (fcntl(fd, F_ADD_SEALS, F_SEAL_SHRINK) < 0) + return NULL; int rc; with_signals_blocked(SIGALRM) From f63b44d8ae68d7b36346d8d00681fe8a84e78ff0 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Sat, 18 Apr 2026 09:13:06 +1000 Subject: [PATCH 09/20] util: allow for a maximum of 32 fds in xsend_with_fds Our current maximum of fds per the protocol is exactly 1 so this has no effect but might save us in the future from some naughty client. Meanwhile, this prevents us from having a variable-length array on the stack that is caller-controller. Assisted-by: Claude:claude-opus-4-6 Part-of: --- src/util-io.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/util-io.c b/src/util-io.c index f3f7af8..acc35f0 100644 --- a/src/util-io.c +++ b/src/util-io.c @@ -88,6 +88,11 @@ xsend_with_fd(int fd, const void *buf, size_t len, int *fds) if (nfds == 0) return xsend(fd, buf, len); + const size_t MAX_FDS = 32; + if (nfds > MAX_FDS) { + return -EINVAL; + } + char control[CMSG_SPACE(nfds * sizeof(int))]; struct cmsghdr *header = (struct cmsghdr *)control; From 80e2b4e4d92badc9b4599d84f2d0f7eb9a305b46 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Fri, 17 Apr 2026 18:49:12 +1000 Subject: [PATCH 10/20] util: close excessive file descriptors in iobuf_recv_from_fd Unceremoneously close any fds that the client gives us that exceed our expectations (which right now is a max of 32 for a protocol-valid max of... 1). Assisted-by: Claude:claude-opus-4-6 Part-of: --- src/util-io.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/util-io.c b/src/util-io.c index acc35f0..3315ab6 100644 --- a/src/util-io.c +++ b/src/util-io.c @@ -390,6 +390,12 @@ iobuf_recv_from_fd(struct iobuf *buf, int fd) fd++; } } + /* Close any remaining fds that didn't fit in the buffer + * to prevent fd leaks */ + while (*fd != -1) { + xclose(*fd); + fd++; + } } nread += rc; From 7d42a95d1206db12ccd2ce7ab2f1e3d483e66ba1 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Fri, 17 Apr 2026 18:50:49 +1000 Subject: [PATCH 11/20] util: replace strcat memcpy in strv_join Repeatedly using strcat means we rescan the string, making it O(n^2). Use memcpy and an offset instead because we can't affort to waste nanoseconds here. Assisted-by: Claude:claude-opus-4-6 Part-of: --- src/util-strings.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/util-strings.c b/src/util-strings.c index 41c1d95..6e6140a 100644 --- a/src/util-strings.c +++ b/src/util-strings.c @@ -150,11 +150,17 @@ strv_join(char **strv, const char *joiner) slen += (count - 1) * strlen(joiner); str = xalloc(slen + 1); /* trailing \0 */ + size_t jlen = strlen(joiner); + size_t offset = 0; for (s = strv; *s; s++) { - strcat(str, *s); + size_t l = strlen(*s); + memcpy(str + offset, *s, l); + offset += l; --count; - if (count > 0) - strcat(str, joiner); + if (count > 0) { + memcpy(str + offset, joiner, jlen); + offset += jlen; + } } return str; From 62ac026432aa0e2623f9627afc5fec6324b35f58 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Mon, 11 May 2026 11:45:09 +1000 Subject: [PATCH 12/20] oeffis: handle fcntl failures correctly Assisted-by: Claude:claude-opus-4-6 Part-of: --- src/liboeffis.c | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/src/liboeffis.c b/src/liboeffis.c index d516650..b1855d8 100644 --- a/src/liboeffis.c +++ b/src/liboeffis.c @@ -327,7 +327,6 @@ xdp_session_path(char *sender_name, char *token) static int connect_to_eis_returned(sd_bus_message *m, void *userdata, sd_bus_error *ret_error) { - int eisfd; struct oeffis *oeffis = userdata; int rc = sd_bus_message_get_errno(m); @@ -336,27 +335,37 @@ connect_to_eis_returned(sd_bus_message *m, void *userdata, sd_bus_error *ret_err return rc; } - rc = sd_bus_message_read(m, "h", &eisfd); + int sd_eisfd; + rc = sd_bus_message_read(m, "h", &sd_eisfd); if (rc < 0) { oeffis_disconnect(oeffis, "Unable to get fd from portal: %s", strerror(-rc)); return -rc; } /* the fd is owned by the message */ - rc = xerrno(xdup(eisfd)); - if (rc < 0) { - oeffis_disconnect(oeffis, "Failed to dup fd: %s", strerror(-rc)); - return -rc; + _cleanup_close_ int eisfd = xerrno(xdup(sd_eisfd)); + if (eisfd < 0) { + oeffis_disconnect(oeffis, "Failed to dup fd: %s", strerror(-eisfd)); + return -eisfd; } else { - eisfd = rc; - int flags = fcntl(eisfd, F_GETFL, 0); - fcntl(eisfd, F_SETFL, flags | O_NONBLOCK); + int flags = xerrno(fcntl(eisfd, F_GETFL, 0)); + if (flags >= 0) + rc = xerrno(fcntl(eisfd, F_SETFL, flags | O_NONBLOCK)); + else + rc = flags; + + if (rc < 0) { + oeffis_disconnect(oeffis, + "Failed to set the fd to non-blocking: %s", + strerror(-rc)); + return -rc; + } } log_debug("Got fd %d from portal", eisfd); - rc = oeffis_set_eis_fd(oeffis, eisfd); + rc = oeffis_set_eis_fd(oeffis, steal_fd(&eisfd)); if (rc < 0) { oeffis_disconnect(oeffis, "Failed to set the fd: %s", strerror(-rc)); return -rc; From c436bd37ec770313c8738a0b33dd6283b20d5093 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Sat, 18 Apr 2026 09:12:11 +1000 Subject: [PATCH 13/20] eis: fix two file descriptor leaks on error Assisted-by: Claude:claude-opus-4-6 Part-of: --- src/libeis-fd.c | 6 +++++- src/libeis-socket.c | 4 ++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/libeis-fd.c b/src/libeis-fd.c index 6ed151e..dcccf0b 100644 --- a/src/libeis-fd.c +++ b/src/libeis-fd.c @@ -28,6 +28,7 @@ #include #include +#include "util-io.h" #include "util-macros.h" #include "util-mem.h" #include "util-sources.h" @@ -83,8 +84,11 @@ eis_backend_fd_add_client(struct eis *eis) return -errno; struct eis_client *client = eis_client_new(eis, fds[0]); - if (client == NULL) + if (client == NULL) { + xclose(fds[0]); + xclose(fds[1]); return -ENOMEM; + } eis_client_unref(client); diff --git a/src/libeis-socket.c b/src/libeis-socket.c index 671b3d5..25199fb 100644 --- a/src/libeis-socket.c +++ b/src/libeis-socket.c @@ -115,6 +115,10 @@ listener_dispatch(struct source *source, void *data) return; struct eis_client *client = eis_client_new(eis, fd); + if (client == NULL) { + xclose(fd); + return; + } eis_client_unref(client); } From 2b9b001db998549348b7c01635908304580b2243 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Fri, 17 Apr 2026 15:42:47 +1000 Subject: [PATCH 14/20] eis: our socket lockfile doesn't need group permissions Assisted-by: Claude:claude-opus-4-6 Part-of: --- src/libeis-socket.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libeis-socket.c b/src/libeis-socket.c index 25199fb..c743836 100644 --- a/src/libeis-socket.c +++ b/src/libeis-socket.c @@ -147,7 +147,7 @@ eis_setup_backend_socket(struct eis *eis, const char *socketpath) * socket file. */ _cleanup_free_ char *lockfile = xaprintf("%s.lock", path); _cleanup_close_ int lockfd = - open(lockfile, O_CREAT | O_CLOEXEC | O_RDWR, S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP); + open(lockfile, O_CREAT | O_CLOEXEC | O_RDWR, S_IRUSR | S_IWUSR); int rc = flock(lockfd, LOCK_EX | LOCK_NB); if (rc < 0) { log_error(eis, "Failed to create lockfile %s, is another EIS running?", lockfile); From c352f16b4a70dbea67902ec1846d3bd2dbb22926 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Fri, 17 Apr 2026 18:55:43 +1000 Subject: [PATCH 15/20] eis: restrict our socket to owner-only Depending on the umask our socket may be group/world-accessible, let's not do that because if the current state of the world tells us anything it is that we can't trust it. Assisted-by: Claude:claude-opus-4-6 Part-of: --- src/libeis-socket.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/libeis-socket.c b/src/libeis-socket.c index c743836..888c955 100644 --- a/src/libeis-socket.c +++ b/src/libeis-socket.c @@ -181,6 +181,10 @@ eis_setup_backend_socket(struct eis *eis, const char *socketpath) if (bind(sockfd, (struct sockaddr *)&addr, sizeof(addr)) == -1) return -errno; + /* Restrict socket to owner-only access regardless of umask */ + if (fchmod(sockfd, S_IRUSR | S_IWUSR) == -1) + return -errno; + if (listen(sockfd, 2) == -1) return -errno; From dc5bd1396fb9cbb25fb7f2fac03a352fe942b1a3 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Mon, 11 May 2026 13:43:50 +1000 Subject: [PATCH 16/20] eis: handle a lock file open failure correctly Assisted-by: Claude:claude-opus-4-6 Part-of: --- src/libeis-socket.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/libeis-socket.c b/src/libeis-socket.c index 888c955..5ce918b 100644 --- a/src/libeis-socket.c +++ b/src/libeis-socket.c @@ -147,11 +147,15 @@ eis_setup_backend_socket(struct eis *eis, const char *socketpath) * socket file. */ _cleanup_free_ char *lockfile = xaprintf("%s.lock", path); _cleanup_close_ int lockfd = - open(lockfile, O_CREAT | O_CLOEXEC | O_RDWR, S_IRUSR | S_IWUSR); - int rc = flock(lockfd, LOCK_EX | LOCK_NB); + xerrno(open(lockfile, O_CREAT | O_CLOEXEC | O_RDWR, S_IRUSR | S_IWUSR)); + int rc; + if (lockfd >= 0) + rc = xerrno(flock(lockfd, LOCK_EX | LOCK_NB)); + else + rc = lockfd; if (rc < 0) { log_error(eis, "Failed to create lockfile %s, is another EIS running?", lockfile); - return -errno; + return -rc; } struct stat st; From 4ec936f016f98e3058a26739beb468caed5607eb Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Sat, 18 Apr 2026 09:21:17 +1000 Subject: [PATCH 17/20] tools: use xatou in the demo client/server Assisted-by: Claude:claude-opus-4-6 Part-of: --- tools/ei-demo-client.c | 10 ++++++++-- tools/eis-demo-server.c | 5 ++++- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/tools/ei-demo-client.c b/tools/ei-demo-client.c index b12de52..16ce9a0 100644 --- a/tools/ei-demo-client.c +++ b/tools/ei-demo-client.c @@ -258,10 +258,16 @@ main(int argc, char **argv) receiver = true; break; case OPT_INTERVAL: - interval = atoi(optarg); + if (!xatou(optarg, &interval)) { + fprintf(stderr, "Invalid interval: %s\n", optarg); + return EXIT_FAILURE; + } break; case OPT_ITERATIONS: - iterations = atoi(optarg); + if (!xatou(optarg, &iterations)) { + fprintf(stderr, "Invalid iterations: %s\n", optarg); + return EXIT_FAILURE; + } break; default: usage(stderr, argv[0]); diff --git a/tools/eis-demo-server.c b/tools/eis-demo-server.c index 9cf04ad..06a9c89 100644 --- a/tools/eis-demo-server.c +++ b/tools/eis-demo-server.c @@ -698,7 +698,10 @@ main(int argc, char **argv) verbose = true; break; case OPT_INTERVAL: - interval = atoi(optarg); + if (!xatou(optarg, &interval)) { + fprintf(stderr, "Invalid interval: %s\n", optarg); + return EXIT_FAILURE; + } break; default: usage(stderr, argv[0]); From 581806d28e4da91e84f73e73af44d7bd708552a1 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Mon, 11 May 2026 12:08:05 +1000 Subject: [PATCH 18/20] tools: verify the libxkbcommon utf8 symbols This is mostly to shut up analyzers that aren't happy with the strcat usage here. Assisted-by: Claude:claude-opus-4-6 Part-of: --- tools/ei-demo-client.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/ei-demo-client.c b/tools/ei-demo-client.c index 16ce9a0..415ecc4 100644 --- a/tools/ei-demo-client.c +++ b/tools/ei-demo-client.c @@ -131,7 +131,8 @@ setup_xkb_keymap(struct ei_keymap *keymap) for (unsigned int evcode = KEY_Q; evcode <= KEY_Y; evcode++) { char utf8[7]; xkb_keysym_t keysym = xkb_state_key_get_one_sym(xkbstate, evcode + 8); - xkb_keysym_to_utf8(keysym, utf8, sizeof(utf8)); + int len = xkb_keysym_to_utf8(keysym, utf8, sizeof(utf8)); + assert(len > 0 && (size_t)len <= sizeof(utf8)); strcat(layout, utf8); } From 93ba922062c415a8416a44e72573dded7bf7d274 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Fri, 17 Apr 2026 18:54:27 +1000 Subject: [PATCH 19/20] brei: fail on under/oversized message lengths Add a 1MiB maximum and make sure our message length is at least the header size - without that we get an underflow, causing iobuf_pop to remove a huge number of bytes, crashing the other end. Assisted-by: Claude:claude-opus-4-6 Part-of: --- src/brei-shared.c | 10 ++++++-- test/test_protocol.py | 58 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 2 deletions(-) diff --git a/src/brei-shared.c b/src/brei-shared.c index 6846bbf..fa1fccd 100644 --- a/src/brei-shared.c +++ b/src/brei-shared.c @@ -396,8 +396,14 @@ brei_dispatch(struct brei_context *brei, const struct brei_header *header = (const struct brei_header *)data; uint32_t msglen = header->msglen; - if (len < msglen) - break; + /* Max message size: 1MiB, plenty enough for the current protocol */ + static const uint32_t max_msglen = 1024 * 1024; + if (msglen < headersize || msglen > max_msglen) { + result = brei_result_new(BREI_CONNECTION_DISCONNECT_REASON_PROTOCOL, + "invalid message length %u", + msglen); + goto error; + } object_id_t object_id = header->sender_id; uint32_t opcode = header->opcode; diff --git a/test/test_protocol.py b/test/test_protocol.py index 60420b3..1d6d61b 100644 --- a/test/test_protocol.py +++ b/test/test_protocol.py @@ -33,6 +33,7 @@ import time import shlex import signal import socket +import struct import structlog try: @@ -1359,3 +1360,60 @@ class TestEiProtocol: ei.wait_for(lambda: status.disconnected) assert status.disconnected is True + + @pytest.mark.parametrize( + "invalid_msglen", + (0, 1, 15, 1024 * 1024 + 1, 0xFFFFFFFF), + ids=("zero", "one", "header-minus-one", "over-max", "uint32-max"), + ) + def test_invalid_message_length(self, eis, invalid_msglen): + """ + Ensure the server disconnects us if we send a message with an invalid + msglen (less than header size or greater than 1MiB). + """ + ei = eis.ei + ei.dispatch() + ei.init_default_sender_connection() + ei.wait_for_connection() + + assert ei.connection is not None + connection = ei.connection + + @dataclass + class Status: + disconnected: bool = False + reason: int = 0 + explanation: Optional[str] = None + + status = Status() + + def on_disconnected(connection, last_serial, reason, explanation): + status.disconnected = True + status.reason = reason + status.explanation = explanation + + connection.connect("Disconnected", on_disconnected) + + # Craft a raw message with a valid object_id (the connection's) but + # an invalid msglen. Use opcode 0 since the msglen check happens + # before opcode validation. + raw_msg = struct.pack("=QII", connection.object_id, invalid_msglen, 0) + # For oversized messages we only need to send the header - the server + # checks msglen from the header before trying to read the full payload. + try: + ei.send(raw_msg) + ei.dispatch() + time.sleep(0.5) + ei.dispatch() + except (ConnectionResetError, BrokenPipeError): + # The server may have already closed the connection + return + + # If we didn't get a socket error, we should have gotten a + # Disconnected event with a protocol error reason + assert status.disconnected, ( + f"Expected disconnection for invalid msglen {invalid_msglen}" + ) + assert status.reason == EiConnection.EiDisconnectReason.PROTOCOL, ( + status.explanation + ) From fc5129968dd0030e711f7f17cd2c897f4d08ead5 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Mon, 11 May 2026 12:18:10 +1000 Subject: [PATCH 20/20] brei: put a warning in that disabling NDEBUG is not a good idea We really rely on assert all over the place, removing that is likely going to result in interesting outcomes when unexpected failures happen. Putting this into brei because it's a header used by both libei/libeis. Assisted-by: Claude:claude-opus-4-6 Part-of: --- src/brei-shared.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/brei-shared.h b/src/brei-shared.h index 6f25d43..d28c505 100644 --- a/src/brei-shared.h +++ b/src/brei-shared.h @@ -26,6 +26,10 @@ #include "config.h" +#ifdef NDEBUG +#warning "This project relies on assert(). #defining NDEBUG is not recommended" +#endif + #include #include #include