From 2baa2791cec4e82c735f9f8d77759b6b66a617b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ferenc=20W=C3=A1gner?= Date: Thu, 18 Apr 2019 16:06:04 +0200 Subject: [PATCH] Let remote_tempdir() assume a NUL-terminated name This is the case already. We also fix a buffer overflow opportunity in the memcpy() call by this change. Conflicts: lib/ipc_shm.c --- lib/ipc_int.h | 2 +- lib/ipc_setup.c | 11 +++++------ lib/ipc_shm.c | 3 +-- lib/ipc_socket.c | 4 ++-- lib/ipcs.c | 2 +- 5 files changed, 10 insertions(+), 12 deletions(-) diff --git a/lib/ipc_int.h b/lib/ipc_int.h index c8904487e..70f7d17a1 100644 --- a/lib/ipc_int.h +++ b/lib/ipc_int.h @@ -208,6 +208,6 @@ int32_t qb_ipc_us_sock_error_is_disconnected(int err); int use_filesystem_sockets(void); -void remove_tempdir(const char *name, size_t namelen); +void remove_tempdir(const char *name); #endif /* QB_IPC_INT_H_DEFINED */ diff --git a/lib/ipc_setup.c b/lib/ipc_setup.c index 45fec8bb8..23eeabb21 100644 --- a/lib/ipc_setup.c +++ b/lib/ipc_setup.c @@ -914,16 +914,15 @@ qb_ipcs_us_connection_acceptor(int fd, int revent, void *data) return 0; } -void remove_tempdir(const char *name, size_t namelen) +void remove_tempdir(const char *name) { #if defined(QB_LINUX) || defined(QB_CYGWIN) char dirname[PATH_MAX]; - char *slash; - memcpy(dirname, name, namelen); + char *slash = strrchr(name, '/'); - slash = strrchr(dirname, '/'); - if (slash) { - *slash = '\0'; + if (slash && slash - name < sizeof dirname) { + memcpy(dirname, name, slash - name); + dirname[slash - name] = '\0'; /* This gets called more than it needs to be really, so we don't check * the return code. It's more of a desperate attempt to clean up after ourself * in either the server or client. diff --git a/lib/ipc_shm.c b/lib/ipc_shm.c index 637a6a7b3..a846fcc45 100644 --- a/lib/ipc_shm.c +++ b/lib/ipc_shm.c @@ -266,10 +266,9 @@ qb_ipcs_shm_disconnect(struct qb_ipcs_connection *c) } } - remove_tempdir(c->description, CONNECTION_DESCRIPTION); - end_disconnect: sigaction(SIGBUS, &old_sa, NULL); + remove_tempdir(c->description); } static int32_t diff --git a/lib/ipc_socket.c b/lib/ipc_socket.c index 48f06dc2b..800f595a9 100644 --- a/lib/ipc_socket.c +++ b/lib/ipc_socket.c @@ -376,7 +376,7 @@ qb_ipcc_us_disconnect(struct qb_ipcc_connection *c) } /* Last-ditch attempt to tidy up after ourself */ - remove_tempdir(c->request.u.us.shared_file_name, PATH_MAX); + remove_tempdir(c->request.u.us.shared_file_name); qb_ipcc_us_sock_close(c->event.u.us.sock); qb_ipcc_us_sock_close(c->request.u.us.sock); @@ -772,7 +772,7 @@ qb_ipcs_us_disconnect(struct qb_ipcs_connection *c) } - remove_tempdir(c->description, CONNECTION_DESCRIPTION); + remove_tempdir(c->description); } static int32_t diff --git a/lib/ipcs.c b/lib/ipcs.c index 29f3431b0..0609e46ac 100644 --- a/lib/ipcs.c +++ b/lib/ipcs.c @@ -642,7 +642,7 @@ qb_ipcs_disconnect(struct qb_ipcs_connection *c) scheduled_retry = 1; } } - remove_tempdir(c->description, CONNECTION_DESCRIPTION); + remove_tempdir(c->description); if (scheduled_retry == 0) { /* This removes the initial alloc ref */ qb_ipcs_connection_unref(c);