[Debian-ha-maintainers] Bug#927159: libqb: CVE-2019-12779: Insecure Temporary Files
wferi at niif.hu
wferi at niif.hu
Sun Jun 16 23:52:54 BST 2019
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 <wferi at 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 at Base 0.11.1
qb_vsnprintf_deserialize at Base 0.11.1
qb_vsnprintf_serialize at Base 0.14.2
+ remove_tempdir at Base 1.0.1-1+deb9u1~
strlcat at Base 0.11.1
strlcpy at 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?= <wferi at 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?= <wferi at 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 <ccaulfie at 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 <ccaulfie at 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?= <wferi at 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?= <wferi at 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
More information about the Debian-ha-maintainers
mailing list