libqb: CVE-2019-12779: Insecure Temporary Files

Debian Bug report logs - #927159
libqb: CVE-2019-12779: Insecure Temporary Files

Reply or subscribe to this bug.

Toggle useless messages

Message #5 received at submit@bugs.debian.org (full text, mbox, reply):

From: Ferenc Wágner <wferi@debian.org>

To: Debian Bug Tracking System <submit@bugs.debian.org>

Subject: libqb: Insecure Temporary Files

Date: Mon, 15 Apr 2019 20:10:07 +0200

Source: libqb
Version: 1.0.3-2
Severity: grave
Tags: patch upstream security
Justification: user security hole
Forwarded: https://github.com/ClusterLabs/libqb/issues/338
Control: found -1 0.11.1-2

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).

Message #12 received at 927159-close@bugs.debian.org (full text, mbox, reply):

From: Ferenc Wágner <wferi@debian.org>

To: 927159-close@bugs.debian.org

Subject: Bug#927159: fixed in libqb 1.0.4-1

Date: Tue, 16 Apr 2019 10:18:54 +0000

Source: libqb
Source-Version: 1.0.4-1

We believe that the bug you reported is fixed in the latest version of
libqb, which is due to be installed in the Debian FTP archive.

A summary of the changes between this version and the previous one is
attached.

Thank you for reporting the bug, which will now be closed.  If you
have further comments please address them to 927159@bugs.debian.org,
and the maintainer will reopen the bug report if appropriate.

Debian distribution maintenance software
pp.
Ferenc Wágner <wferi@debian.org> (supplier of updated libqb package)

(This message was generated automatically at their request; if you
believe that there is a problem with it please contact the archive
administrators by mailing ftpmaster@ftp-master.debian.org)


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

Format: 1.8
Date: Tue, 16 Apr 2019 11:32:02 +0200
Source: libqb
Architecture: source
Version: 1.0.4-1
Distribution: unstable
Urgency: high
Maintainer: Debian HA Maintainers <debian-ha-maintainers@lists.alioth.debian.org>
Changed-By: Ferenc Wágner <wferi@debian.org>
Closes: 902104 927159
Changes:
 libqb (1.0.4-1) unstable; urgency=high
 .
   * [021333b] New patch: Reduce stress test lengths to help weak buildds
     (Closes: #902104)
   * [dc825fb] New upstream security release (1.0.4)
     Libqb creates files in world-writable directories (/dev/shm, /tmp) with
     rather predictable file names. Also O_EXCL flag is not used when opening
     the files.  This could be exploited by a local attacker to overwrite
     privileged system files.  (CVE not assigned yet. Closes: #927159)
   * [e36e34a] Refresh our patches
   * [0823bf1] Acknowledge new internal symbol
   * [88aafa5] Update Standards-Version to 4.3.0 (no changes required)
   * [174d210] New patch: Fix garbled Doxygen markup
Checksums-Sha1:
 710f93c4bac969ce0ae0796ddb484637e016e3ef 2423 libqb_1.0.4-1.dsc
 b8078893be6c2c355313fe71cf006da7713ea43e 488536 libqb_1.0.4.orig.tar.xz
 a0c36c968b98ff62fb2e08d1676068f074bc22da 490 libqb_1.0.4.orig.tar.xz.asc
 597140cbb986bae40feabd146485cac13b732061 14660 libqb_1.0.4-1.debian.tar.xz
 7ad39ccc76987a16c2557887a4ef38d59fd17c74 8994 libqb_1.0.4-1_amd64.buildinfo
Checksums-Sha256:
 f6baa06c325116c25dfb6e268b1f26c4c0cbe1440c5466e8bb15aa31a5f4d291 2423 libqb_1.0.4-1.dsc
 0064575151b11135a68a15f01da0ab1eaef4279b07e7059a10c748c12f28c14f 488536 libqb_1.0.4.orig.tar.xz
 10029fa1da3752b9c28ac2a7530108671a084fa80f37c5b211aeec2de74744ae 490 libqb_1.0.4.orig.tar.xz.asc
 23c9517fb3f746f50ce7bd99deb48d03ab88521ba1801ceeb2a501dd54311fae 14660 libqb_1.0.4-1.debian.tar.xz
 4c61f83a9a0fa365561fb7ddd2300f5ed09cf1dcef1fa763eba658b46d40c69a 8994 libqb_1.0.4-1_amd64.buildinfo
Files:
 254a63658735dcb207be357b4c2b3e6c 2423 libs optional libqb_1.0.4-1.dsc
 07dc93433a295f45dd16293a4b63389e 488536 libs optional libqb_1.0.4.orig.tar.xz
 22d567fc789da092b0e14e1c9d625ac5 490 libs optional libqb_1.0.4.orig.tar.xz.asc
 61a49b2f90a1bbe677275238cd107213 14660 libs optional libqb_1.0.4-1.debian.tar.xz
 3d70e4dc9e8d7c4d71204c670a3e11c5 8994 libs optional libqb_1.0.4-1_amd64.buildinfo

-----BEGIN PGP SIGNATURE-----

iQIyBAEBCgAdFiEEwddEx0RNIUL7eugtOsj3Fkd+2yMFAly1pF0ACgkQOsj3Fkd+
2yM2Bw/2JzHPYzWM71U7xw9/rYefEfzigmRTofej16IQtx4dGEmGs5wy2lkDrY/Y
lmho1aPv8dpV46Xz4oTGhi9n1L02b9hD3eVJ0CsJ6YCFQaBJ9CcXPM+TQSLCjYN8
syGl/jqKknAfRNJG50NnQUXnACHmgJ79CVaXyiZh432V28kOYJBgGJi0ayqznd9r
G8eHoYYdmk0zV+SdaQ6fTeg8pitu6QWLHQ73qfVlaLO+uCeRZUgJl8Ah9raHPqgi
DA7OPoPWdlsrQc0kRKI3vQ0KMkphjzsymy+TsXD0zomXp7X6tJ97rx1q3yylO528
lX86UNll+aY84dejFu+LxJG7UBFW1ADlgd830qEUVR0SbL97pVKZCwwJ4k/Z9zN0
I7c3wFAwUJF/XkIc3serOjUjj3AB7XvKuQWXNHbaN6e2IurtrRrn3SRKrp9WX5l7
/5uyaan3cZ1mugphn1cSStyw2+nqOPp5SyeqTRkhL1LkTISpvmm24/QQGOvGrv2z
vPgy7OC1AwQroagp0DjwCSLyPWdJm7Svaic4WmzNaYmUF/PmytW4sdNboCC1ZAGK
cNhrisnYd8yLH+geGKIfhrXOXJ7ivcoufFHpBROnoazhVtF+gGOm5k4HqYaPOVb5
3PklhlTgilvG/9wY3662YIHWQIKFd+gPth/dfKjfexoHNJl94Q==
=NWod
-----END PGP SIGNATURE-----

Message #19 received at 927159@bugs.debian.org (full text, mbox, reply):

From: wferi@niif.hu

To: 927159@bugs.debian.org

Cc: team@security.debian.org, wferi@niif.hu

Subject: Re: libqb: CVE-2019-12779: Insecure Temporary Files

Date: Mon, 17 Jun 2019 00:52:54 +0200

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@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?= <wferi@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@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@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@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@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@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

Message #24 received at 927159@bugs.debian.org (full text, mbox, reply):

From: Moritz Muehlenhoff <jmm@inutil.org>

To: wferi@niif.hu

Cc: 927159@bugs.debian.org, team@security.debian.org

Subject: Re: libqb: CVE-2019-12779: Insecure Temporary Files

Date: Mon, 17 Jun 2019 09:07:56 +0200

On Mon, Jun 17, 2019 at 12:52:54AM +0200, wferi@niif.hu wrote:
> 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)

Debian enables fs.protected_symlinks by default since Jessie and running it
without this options is as an unsupported configuration (i.e. if anyone builds
their own kernel, they also need to enable it). This can be fixed via a point
release still, but it doesn't warrant a DSA.

Cheers,
        Moritz

Send a report that this bug log contains spam.