Dear Security Team, I'm ready to upload libqb-1.0.1-1+deb9u1 with the following debdiff:
diff -Nru libqb-1.0.1/debian/changelog libqb-1.0.1/debian/changelog --- libqb-1.0.1/debian/changelog 2016-12-07 14:55:45.000000000 +0100 +++ libqb-1.0.1/debian/changelog 2019-06-16 23:41:50.000000000 +0200 @@ -1,3 +1,21 @@ +libqb (1.0.1-1+deb9u1) stretch-security; urgency=high + + * [38e0e13] Backport upstream security fixes for CVE-2019-12779. + Libqb creates files in world-writable directories (/dev/shm, /tmp) with + rather predictable file names (for example in case of USBGuard with names + like /dev/shm/qb-usbguard-request-7096-835-12-data). Also O_EXCL flag is + not used when opening the files. This could be exploited by a local + attacker to overwrite privileged system files (if not restricted by + sandboxing, MAC or symlinking policies). + Original report: https://github.com/ClusterLabs/libqb/issues/338 + Add O_EXCL: https://github.com/ClusterLabs/libqb/pull/339 + Use mkdtemp(): https://github.com/ClusterLabs/libqb/pull/345 + Regression fixes: https://github.com/ClusterLabs/libqb/pull/349 + (Closes: #927159) + * [79734d7] Acknowledge new internal symbol + + -- Ferenc Wágner <wf...@debian.org> Sun, 16 Jun 2019 23:41:50 +0200 + libqb (1.0.1-1) unstable; urgency=medium [ Christoph Berg ] diff -Nru libqb-1.0.1/debian/gbp.conf libqb-1.0.1/debian/gbp.conf --- libqb-1.0.1/debian/gbp.conf 2016-12-07 14:47:16.000000000 +0100 +++ libqb-1.0.1/debian/gbp.conf 2019-06-16 22:48:22.000000000 +0200 @@ -1,14 +1,12 @@ [DEFAULT] -debian-branch = debian/master +debian-branch = debian/stretch upstream-branch = upstream/latest -debian-tag-msg = Debian release %(version)s - -[import-orig] pristine-tar = True -[gbp-pq] -patch-numbers = False - -[gbp-dch] +[dch] +full = True multimaint-merge = True id-length = 7 + +[pq] +patch-numbers = False diff -Nru libqb-1.0.1/debian/libqb0.symbols libqb-1.0.1/debian/libqb0.symbols --- libqb-1.0.1/debian/libqb0.symbols 2016-12-07 14:47:16.000000000 +0100 +++ libqb-1.0.1/debian/libqb0.symbols 2019-06-16 23:41:22.000000000 +0200 @@ -236,5 +236,6 @@ qb_util_timespec_from_epoch_get@Base 0.11.1 qb_vsnprintf_deserialize@Base 0.11.1 qb_vsnprintf_serialize@Base 0.14.2 + remove_tempdir@Base 1.0.1-1+deb9u1~ strlcat@Base 0.11.1 strlcpy@Base 0.11.1 diff -Nru libqb-1.0.1/debian/patches/CVE-2019-12779/Allow-group-access-to-the-IPC-directory.patch libqb-1.0.1/debian/patches/CVE-2019-12779/Allow-group-access-to-the-IPC-directory.patch --- libqb-1.0.1/debian/patches/CVE-2019-12779/Allow-group-access-to-the-IPC-directory.patch 1970-01-01 01:00:00.000000000 +0100 +++ libqb-1.0.1/debian/patches/CVE-2019-12779/Allow-group-access-to-the-IPC-directory.patch 2019-06-16 23:10:03.000000000 +0200 @@ -0,0 +1,29 @@ +From: =?utf-8?q?Ferenc_W=C3=A1gner?= <wf...@debian.org> +Date: Thu, 18 Apr 2019 13:20:38 +0200 +Subject: Allow group access to the IPC directory + +And don't abort if we aren't permitted to chown() it. The client might +still have the privileges to enter it. +--- + lib/ipc_setup.c | 5 +++-- + 1 file changed, 3 insertions(+), 2 deletions(-) + +diff --git a/lib/ipc_setup.c b/lib/ipc_setup.c +index f6f1d7d..6183001 100644 +--- a/lib/ipc_setup.c ++++ b/lib/ipc_setup.c +@@ -640,11 +640,12 @@ handle_new_connection(struct qb_ipcs_service *s, + res = -errno; + goto send_response; + } +- res = chown(c->description, c->auth.uid, c->auth.gid); +- if (res != 0) { ++ if (chmod(c->description, 0770)) { + res = -errno; + goto send_response; + } ++ /* chown can fail because we might not be root */ ++ (void)chown(c->description, c->auth.uid, c->auth.gid); + + /* We can't pass just a directory spec to the clients */ + strncat(c->description,"/qb", CONNECTION_DESCRIPTION); diff -Nru libqb-1.0.1/debian/patches/CVE-2019-12779/Errors-are-represented-as-negative-values.patch libqb-1.0.1/debian/patches/CVE-2019-12779/Errors-are-represented-as-negative-values.patch --- libqb-1.0.1/debian/patches/CVE-2019-12779/Errors-are-represented-as-negative-values.patch 1970-01-01 01:00:00.000000000 +0100 +++ libqb-1.0.1/debian/patches/CVE-2019-12779/Errors-are-represented-as-negative-values.patch 2019-06-16 23:10:03.000000000 +0200 @@ -0,0 +1,27 @@ +From: =?utf-8?q?Ferenc_W=C3=A1gner?= <wf...@debian.org> +Date: Wed, 17 Apr 2019 15:09:42 +0200 +Subject: Errors are represented as negative values + +--- + lib/ipc_setup.c | 4 ++-- + 1 file changed, 2 insertions(+), 2 deletions(-) + +diff --git a/lib/ipc_setup.c b/lib/ipc_setup.c +index a66004d..f6f1d7d 100644 +--- a/lib/ipc_setup.c ++++ b/lib/ipc_setup.c +@@ -637,12 +637,12 @@ handle_new_connection(struct qb_ipcs_service *s, + snprintf(c->description, CONNECTION_DESCRIPTION, + "/dev/shm/qb-%d-%d-%d-XXXXXX", s->pid, ugp->pid, c->setup.u.us.sock); + if (mkdtemp(c->description) == NULL) { +- res = errno; ++ res = -errno; + goto send_response; + } + res = chown(c->description, c->auth.uid, c->auth.gid); + if (res != 0) { +- res = errno; ++ res = -errno; + goto send_response; + } + diff -Nru libqb-1.0.1/debian/patches/CVE-2019-12779/ipc-Use-mkdtemp-for-more-secure-IPC-files.patch libqb-1.0.1/debian/patches/CVE-2019-12779/ipc-Use-mkdtemp-for-more-secure-IPC-files.patch --- libqb-1.0.1/debian/patches/CVE-2019-12779/ipc-Use-mkdtemp-for-more-secure-IPC-files.patch 1970-01-01 01:00:00.000000000 +0100 +++ libqb-1.0.1/debian/patches/CVE-2019-12779/ipc-Use-mkdtemp-for-more-secure-IPC-files.patch 2019-06-16 23:10:03.000000000 +0200 @@ -0,0 +1,226 @@ +From: Christine Caulfield <ccaul...@redhat.com> +Date: Mon, 8 Apr 2019 16:24:19 +0100 +Subject: ipc: Use mkdtemp for more secure IPC files + +Use mkdtemp makes sure that IPC files are only visible to the +owning (client) process and do not use predictable names outside +of that. + +This is not meant to be the last word on the subject, it's mainly a +simple way of making the current libqb more secure. Importantly, it's +backwards compatible with an old server. + +It calls rmdir on the directory created by mkdtemp way too often, but +it seems to be the only way to be sure that things get cleaned up on +the various types of server/client exit. I'm sure we can come up with +something tidier for master but I hope this, or something similar, will +be OK for 1.0.x. +--- + lib/ipc_int.h | 4 +++- + lib/ipc_setup.c | 39 +++++++++++++++++++++++++++++++++++++++ + lib/ipc_shm.c | 8 +++++--- + lib/ipc_socket.c | 13 ++++++++++--- + lib/ipcs.c | 3 ++- + lib/ringbuffer.c | 4 ++-- + lib/unix.c | 4 +++- + 7 files changed, 64 insertions(+), 11 deletions(-) + +diff --git a/lib/ipc_int.h b/lib/ipc_int.h +index f63e053..a5c3e56 100644 +--- a/lib/ipc_int.h ++++ b/lib/ipc_int.h +@@ -160,7 +160,7 @@ enum qb_ipcs_connection_state { + QB_IPCS_CONNECTION_SHUTTING_DOWN, + }; + +-#define CONNECTION_DESCRIPTION (34) /* INT_MAX length + 3 */ ++#define CONNECTION_DESCRIPTION NAME_MAX + + struct qb_ipcs_connection_auth { + uid_t uid; +@@ -205,4 +205,6 @@ int32_t qb_ipcs_process_request(struct qb_ipcs_service *s, + + int32_t qb_ipc_us_sock_error_is_disconnected(int err); + ++void remove_tempdir(const char *name, size_t namelen); ++ + #endif /* QB_IPC_INT_H_DEFINED */ +diff --git a/lib/ipc_setup.c b/lib/ipc_setup.c +index 1724fb2..a66004d 100644 +--- a/lib/ipc_setup.c ++++ b/lib/ipc_setup.c +@@ -632,8 +632,28 @@ handle_new_connection(struct qb_ipcs_service *s, + c->auth.gid = c->egid = ugp->gid; + c->auth.mode = 0600; + c->stats.client_pid = ugp->pid; ++ ++#if defined(QB_LINUX) || defined(QB_CYGWIN) ++ snprintf(c->description, CONNECTION_DESCRIPTION, ++ "/dev/shm/qb-%d-%d-%d-XXXXXX", s->pid, ugp->pid, c->setup.u.us.sock); ++ if (mkdtemp(c->description) == NULL) { ++ res = errno; ++ goto send_response; ++ } ++ res = chown(c->description, c->auth.uid, c->auth.gid); ++ if (res != 0) { ++ res = errno; ++ goto send_response; ++ } ++ ++ /* We can't pass just a directory spec to the clients */ ++ strncat(c->description,"/qb", CONNECTION_DESCRIPTION); ++#else + snprintf(c->description, CONNECTION_DESCRIPTION, + "%d-%d-%d", s->pid, ugp->pid, c->setup.u.us.sock); ++#endif ++ ++ + + if (auth_result == 0 && c->service->serv_fns.connection_accept) { + res = c->service->serv_fns.connection_accept(c, +@@ -854,3 +874,22 @@ retry_accept: + qb_ipcs_uc_recv_and_auth(new_fd, s); + return 0; + } ++ ++void remove_tempdir(const char *name, size_t namelen) ++{ ++#if defined(QB_LINUX) || defined(QB_CYGWIN) ++ char dirname[PATH_MAX]; ++ char *slash; ++ memcpy(dirname, name, namelen); ++ ++ slash = strrchr(dirname, '/'); ++ if (slash) { ++ *slash = '\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. ++ */ ++ (void)rmdir(dirname); ++ } ++#endif ++} +diff --git a/lib/ipc_shm.c b/lib/ipc_shm.c +index 699f4e4..bdd0a0d 100644 +--- a/lib/ipc_shm.c ++++ b/lib/ipc_shm.c +@@ -239,6 +239,8 @@ qb_ipcs_shm_disconnect(struct qb_ipcs_connection *c) + qb_rb_close(qb_rb_lastref_and_ret(&c->request.u.shm.rb)); + } + } ++ ++ remove_tempdir(c->description, CONNECTION_DESCRIPTION); + } + + static int32_t +@@ -285,11 +287,11 @@ qb_ipcs_shm_connect(struct qb_ipcs_service *s, + qb_util_log(LOG_DEBUG, "connecting to client [%d]", c->pid); + + snprintf(r->request, NAME_MAX, "%s-request-%s", +- s->name, c->description); ++ c->description, s->name); + snprintf(r->response, NAME_MAX, "%s-response-%s", +- s->name, c->description); ++ c->description, s->name); + snprintf(r->event, NAME_MAX, "%s-event-%s", +- s->name, c->description); ++ c->description, s->name); + + res = qb_ipcs_shm_rb_open(c, &c->request, + r->request); +diff --git a/lib/ipc_socket.c b/lib/ipc_socket.c +index 8aaa420..22fbb95 100644 +--- a/lib/ipc_socket.c ++++ b/lib/ipc_socket.c +@@ -346,6 +346,10 @@ qb_ipcc_us_disconnect(struct qb_ipcc_connection *c) + unlink(sock_name); + } + #endif ++ ++ /* Last-ditch attempt to tidy up after ourself */ ++ remove_tempdir(c->request.u.us.shared_file_name, PATH_MAX); ++ + qb_ipcc_us_sock_close(c->event.u.us.sock); + qb_ipcc_us_sock_close(c->request.u.us.sock); + qb_ipcc_us_sock_close(c->setup.u.us.sock); +@@ -726,7 +730,10 @@ qb_ipcs_us_disconnect(struct qb_ipcs_connection *c) + c->state == QB_IPCS_CONNECTION_ACTIVE) { + munmap(c->request.u.us.shared_data, SHM_CONTROL_SIZE); + unlink(c->request.u.us.shared_file_name); ++ ++ + } ++ remove_tempdir(c->description, CONNECTION_DESCRIPTION); + } + + static int32_t +@@ -745,9 +752,9 @@ qb_ipcs_us_connect(struct qb_ipcs_service *s, + c->request.u.us.sock = c->setup.u.us.sock; + c->response.u.us.sock = c->setup.u.us.sock; + +- snprintf(r->request, NAME_MAX, "qb-%s-control-%s", +- s->name, c->description); +- snprintf(r->response, NAME_MAX, "qb-%s-%s", s->name, c->description); ++ snprintf(r->request, NAME_MAX, "%s-control-%s", ++ c->description, s->name); ++ snprintf(r->response, NAME_MAX, "%s-%s", c->description, s->name); + + fd_hdr = qb_sys_mmap_file_open(path, r->request, + SHM_CONTROL_SIZE, +diff --git a/lib/ipcs.c b/lib/ipcs.c +index 4a375fc..29f3431 100644 +--- a/lib/ipcs.c ++++ b/lib/ipcs.c +@@ -642,12 +642,13 @@ qb_ipcs_disconnect(struct qb_ipcs_connection *c) + scheduled_retry = 1; + } + } +- ++ remove_tempdir(c->description, CONNECTION_DESCRIPTION); + if (scheduled_retry == 0) { + /* This removes the initial alloc ref */ + qb_ipcs_connection_unref(c); + } + } ++ + } + + static void +diff --git a/lib/ringbuffer.c b/lib/ringbuffer.c +index dde95dd..81a3cba 100644 +--- a/lib/ringbuffer.c ++++ b/lib/ringbuffer.c +@@ -166,7 +166,7 @@ qb_rb_open_2(const char *name, size_t size, uint32_t flags, + /* + * Create a shared_hdr memory segment for the header. + */ +- snprintf(filename, PATH_MAX, "qb-%s-header", name); ++ snprintf(filename, PATH_MAX, "%s-header", name); + fd_hdr = qb_sys_mmap_file_open(path, filename, + shared_size, file_flags); + if (fd_hdr < 0) { +@@ -217,7 +217,7 @@ qb_rb_open_2(const char *name, size_t size, uint32_t flags, + * They have to be separate. + */ + if (flags & QB_RB_FLAG_CREATE) { +- snprintf(filename, PATH_MAX, "qb-%s-data", name); ++ snprintf(filename, PATH_MAX, "%s-data", name); + fd_data = qb_sys_mmap_file_open(path, + filename, + real_size, file_flags); +diff --git a/lib/unix.c b/lib/unix.c +index c31145e..643f361 100644 +--- a/lib/unix.c ++++ b/lib/unix.c +@@ -81,7 +81,9 @@ qb_sys_mmap_file_open(char *path, const char *file, size_t bytes, + (void)strlcpy(path, file, PATH_MAX); + } else { + #if defined(QB_LINUX) || defined(QB_CYGWIN) || defined(QB_GNU) +- snprintf(path, PATH_MAX, "/dev/shm/%s", file); ++ /* This is only now called when talking to an old libqb ++ where we need to add qb- to the name */ ++ snprintf(path, PATH_MAX, "/dev/shm/qb-%s", file); + #else + snprintf(path, PATH_MAX, "%s/%s", SOCKETDIR, file); + is_absolute = path; diff -Nru libqb-1.0.1/debian/patches/CVE-2019-12779/ipc-use-O_EXCL-when-opening-IPC-files.patch libqb-1.0.1/debian/patches/CVE-2019-12779/ipc-use-O_EXCL-when-opening-IPC-files.patch --- libqb-1.0.1/debian/patches/CVE-2019-12779/ipc-use-O_EXCL-when-opening-IPC-files.patch 1970-01-01 01:00:00.000000000 +0100 +++ libqb-1.0.1/debian/patches/CVE-2019-12779/ipc-use-O_EXCL-when-opening-IPC-files.patch 2019-06-16 23:10:03.000000000 +0200 @@ -0,0 +1,35 @@ +From: Christine Caulfield <ccaul...@redhat.com> +Date: Mon, 8 Apr 2019 13:31:38 +0100 +Subject: ipc: use O_EXCL when opening IPC files + +--- + lib/ipc_socket.c | 2 +- + lib/ringbuffer.c | 2 +- + 2 files changed, 2 insertions(+), 2 deletions(-) + +diff --git a/lib/ipc_socket.c b/lib/ipc_socket.c +index 09981b4..8aaa420 100644 +--- a/lib/ipc_socket.c ++++ b/lib/ipc_socket.c +@@ -751,7 +751,7 @@ qb_ipcs_us_connect(struct qb_ipcs_service *s, + + fd_hdr = qb_sys_mmap_file_open(path, r->request, + SHM_CONTROL_SIZE, +- O_CREAT | O_TRUNC | O_RDWR); ++ O_CREAT | O_TRUNC | O_RDWR | O_EXCL); + if (fd_hdr < 0) { + res = fd_hdr; + errno = -fd_hdr; +diff --git a/lib/ringbuffer.c b/lib/ringbuffer.c +index 60b0ea1..dde95dd 100644 +--- a/lib/ringbuffer.c ++++ b/lib/ringbuffer.c +@@ -155,7 +155,7 @@ qb_rb_open_2(const char *name, size_t size, uint32_t flags, + sizeof(struct qb_ringbuffer_shared_s) + shared_user_data_size; + + if (flags & QB_RB_FLAG_CREATE) { +- file_flags |= O_CREAT | O_TRUNC; ++ file_flags |= O_CREAT | O_TRUNC | O_EXCL; + } + + rb = calloc(1, sizeof(struct qb_ringbuffer_s)); diff -Nru libqb-1.0.1/debian/patches/CVE-2019-12779/Let-remote_tempdir-assume-a-NUL-terminated-name.patch libqb-1.0.1/debian/patches/CVE-2019-12779/Let-remote_tempdir-assume-a-NUL-terminated-name.patch --- libqb-1.0.1/debian/patches/CVE-2019-12779/Let-remote_tempdir-assume-a-NUL-terminated-name.patch 1970-01-01 01:00:00.000000000 +0100 +++ libqb-1.0.1/debian/patches/CVE-2019-12779/Let-remote_tempdir-assume-a-NUL-terminated-name.patch 2019-06-16 23:10:03.000000000 +0200 @@ -0,0 +1,100 @@ +From: =?utf-8?q?Ferenc_W=C3=A1gner?= <wf...@debian.org> +Date: Thu, 18 Apr 2019 16:06:04 +0200 +Subject: 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. +--- + lib/ipc_int.h | 2 +- + lib/ipc_setup.c | 11 +++++------ + lib/ipc_shm.c | 2 +- + lib/ipc_socket.c | 4 ++-- + lib/ipcs.c | 2 +- + 5 files changed, 10 insertions(+), 11 deletions(-) + +diff --git a/lib/ipc_int.h b/lib/ipc_int.h +index a5c3e56..d3dde49 100644 +--- a/lib/ipc_int.h ++++ b/lib/ipc_int.h +@@ -205,6 +205,6 @@ int32_t qb_ipcs_process_request(struct qb_ipcs_service *s, + + int32_t qb_ipc_us_sock_error_is_disconnected(int err); + +-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 070f9d3..b660db4 100644 +--- a/lib/ipc_setup.c ++++ b/lib/ipc_setup.c +@@ -894,16 +894,15 @@ retry_accept: + 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 bdd0a0d..41906cb 100644 +--- a/lib/ipc_shm.c ++++ b/lib/ipc_shm.c +@@ -240,7 +240,7 @@ qb_ipcs_shm_disconnect(struct qb_ipcs_connection *c) + } + } + +- remove_tempdir(c->description, CONNECTION_DESCRIPTION); ++ remove_tempdir(c->description); + } + + static int32_t +diff --git a/lib/ipc_socket.c b/lib/ipc_socket.c +index 22fbb95..81bb53d 100644 +--- a/lib/ipc_socket.c ++++ b/lib/ipc_socket.c +@@ -348,7 +348,7 @@ qb_ipcc_us_disconnect(struct qb_ipcc_connection *c) + #endif + + /* 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); +@@ -733,7 +733,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 29f3431..0609e46 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); diff -Nru libqb-1.0.1/debian/patches/CVE-2019-12779/Make-it-impossible-to-truncate-or-overflow-the-connection.patch libqb-1.0.1/debian/patches/CVE-2019-12779/Make-it-impossible-to-truncate-or-overflow-the-connection.patch --- libqb-1.0.1/debian/patches/CVE-2019-12779/Make-it-impossible-to-truncate-or-overflow-the-connection.patch 1970-01-01 01:00:00.000000000 +0100 +++ libqb-1.0.1/debian/patches/CVE-2019-12779/Make-it-impossible-to-truncate-or-overflow-the-connection.patch 2019-06-16 23:10:03.000000000 +0200 @@ -0,0 +1,72 @@ +From: =?utf-8?q?Ferenc_W=C3=A1gner?= <wf...@debian.org> +Date: Thu, 18 Apr 2019 14:32:46 +0200 +Subject: Make it impossible to truncate or overflow the connection description + +It's hard to predict the length of formatted output, so we'd better +notice (and abort) if the description is truncated. Incidentally, +mkdtemp() does this for us in the shared memory branch, but do an +explicit check there as well for consistency, and get rid of the wrongly +parametrized strncat() risking a buffer overflow (CONNECTION_DESCRIPTION +is not the length of the source "/qb"). + +Similar truncation checks should be added to qb_ipcs_{shm,us}_connect() +where they build the request/response names, and possibly to other +places using snprintf(). +--- + lib/ipc_setup.c | 28 +++++++++++++++++++++++----- + 1 file changed, 23 insertions(+), 5 deletions(-) + +diff --git a/lib/ipc_setup.c b/lib/ipc_setup.c +index 6183001..070f9d3 100644 +--- a/lib/ipc_setup.c ++++ b/lib/ipc_setup.c +@@ -610,6 +610,8 @@ handle_new_connection(struct qb_ipcs_service *s, + int32_t res2 = 0; + uint32_t max_buffer_size = QB_MAX(req->max_msg_size, s->max_buffer_size); + struct qb_ipc_connection_response response; ++ const char suffix[] = "/qb"; ++ int desc_len; + + c = qb_ipcs_connection_alloc(s); + if (c == NULL) { +@@ -634,8 +636,16 @@ handle_new_connection(struct qb_ipcs_service *s, + c->stats.client_pid = ugp->pid; + + #if defined(QB_LINUX) || defined(QB_CYGWIN) +- snprintf(c->description, CONNECTION_DESCRIPTION, +- "/dev/shm/qb-%d-%d-%d-XXXXXX", s->pid, ugp->pid, c->setup.u.us.sock); ++ desc_len = snprintf(c->description, CONNECTION_DESCRIPTION - sizeof suffix, ++ "/dev/shm/qb-%d-%d-%d-XXXXXX", s->pid, ugp->pid, c->setup.u.us.sock); ++ if (desc_len < 0) { ++ res = -errno; ++ goto send_response; ++ } ++ if (desc_len >= CONNECTION_DESCRIPTION - sizeof suffix) { ++ res = -ENAMETOOLONG; ++ goto send_response; ++ } + if (mkdtemp(c->description) == NULL) { + res = -errno; + goto send_response; +@@ -648,10 +658,18 @@ handle_new_connection(struct qb_ipcs_service *s, + (void)chown(c->description, c->auth.uid, c->auth.gid); + + /* We can't pass just a directory spec to the clients */ +- strncat(c->description,"/qb", CONNECTION_DESCRIPTION); ++ memcpy(c->description + desc_len, suffix, sizeof suffix); + #else +- snprintf(c->description, CONNECTION_DESCRIPTION, +- "%d-%d-%d", s->pid, ugp->pid, c->setup.u.us.sock); ++ desc_len = snprintf(c->description, CONNECTION_DESCRIPTION, ++ "%d-%d-%d", s->pid, ugp->pid, c->setup.u.us.sock); ++ if (desc_len < 0) { ++ res = -errno; ++ goto send_response; ++ } ++ if (desc_len >= CONNECTION_DESCRIPTION) { ++ res = -ENAMETOOLONG; ++ goto send_response; ++ } + #endif + + diff -Nru libqb-1.0.1/debian/patches/series libqb-1.0.1/debian/patches/series --- libqb-1.0.1/debian/patches/series 2016-12-07 14:54:25.000000000 +0100 +++ libqb-1.0.1/debian/patches/series 2019-06-16 23:10:03.000000000 +0200 @@ -9,3 +9,9 @@ Restrict-pthreads-to-where-it-s-actually-needed.patch Restrict-socket-lib-to-where-it-s-actually-needed.patch Restrict-nsl-lib-to-where-it-s-actually-needed.patch +CVE-2019-12779/ipc-use-O_EXCL-when-opening-IPC-files.patch +CVE-2019-12779/ipc-Use-mkdtemp-for-more-secure-IPC-files.patch +CVE-2019-12779/Errors-are-represented-as-negative-values.patch +CVE-2019-12779/Allow-group-access-to-the-IPC-directory.patch +CVE-2019-12779/Make-it-impossible-to-truncate-or-overflow-the-connection.patch +CVE-2019-12779/Let-remote_tempdir-assume-a-NUL-terminated-name.patch It would be best released together with pacemaker_1.1.16-1+deb9u1 (already in your review queue), because this libqb security upgrade requires a restart of the full cluster stack (corosync and pacemaker) anyway to replace all the vulnerable library code. Such restarts may mean temporary service degradation, so the fewer the better. -- Regards, Feri
