[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: squeeze update of ldb also needed (was: squeeze update of samba?)



On Mon, Dec 21, 2015 at 12:33:40PM +0100, Santiago Ruano Rincón wrote:
> Hi,
> 
> El 18/12/15 a las 00:07, Jelmer Vernooij escribió:
> ...
> > 
> > Thanks for the e-mail. I don't think I will have time to look at
> > updating squeeze in the near future, so it would be great if somebody
> > from the LTS team could take care of that.
> 
> Thanks for you answer, Jelmer. I'd like to take care of this samba
> update.
> 
> Also, to note here, that ldb also needs an update to solve two CVEs:
> https://security-tracker.debian.org/tracker/CVE-2015-3223
> https://security-tracker.debian.org/tracker/CVE-2015-5330
> 
> Jelmer, should the LTS Team take care of these too?

Yes, these are part of the same set of CVEs. Please find attached my
proposed patch for Jessie.

Cheers,

Jelmer
diff -Nru ldb-1.1.17/debian/changelog ldb-1.1.17/debian/changelog
--- ldb-1.1.17/debian/changelog	2015-02-23 18:03:45.000000000 +0000
+++ ldb-1.1.17/debian/changelog	2015-12-17 22:18:52.000000000 +0000
@@ -1,3 +1,10 @@
+ldb (2:1.1.17-2+deb8u1) jessie-security; urgency=high
+
+  * Add patch CVE-2015-3223: Fixes CVE-2015-3223: Denial of Service.
+  * Add patch CVE-2015-5330: Fixes CVE-2015-5330: Remote memory read.
+
+ -- Jelmer Vernooij <jelmer@debian.org>  Wed, 16 Dec 2015 01:33:20 +0000
+
 ldb (2:1.1.17-2) unstable; urgency=high
 
   * No changes from 1:1.1.17-1 (currently in jessie)
diff -Nru ldb-1.1.17/debian/patches/01_exclude_symbols ldb-1.1.17/debian/patches/01_exclude_symbols
--- ldb-1.1.17/debian/patches/01_exclude_symbols	2015-02-23 18:03:45.000000000 +0000
+++ ldb-1.1.17/debian/patches/01_exclude_symbols	1970-01-01 00:00:00.000000000 +0000
@@ -1,24 +0,0 @@
-Description: Properly hide private symbols in libldb1
-Author: Jelmer Vernooij <jelmer@debian.org>
-Bug: https://bugzilla.samba.org/show_bug.cgi?id=9357
-Status: submitted upstream
-
-diff -ur ldb-1.1.13/buildtools/wafsamba/samba_abi.py ldb-1.1.13-fixd/buildtools/wafsamba/samba_abi.py
---- ldb-1.1.13/buildtools/wafsamba/samba_abi.py	2012-09-07 16:47:16.000000000 +0200
-+++ ldb-1.1.13-fixd/buildtools/wafsamba/samba_abi.py	2012-11-05 19:23:40.010206344 +0100
-@@ -190,12 +190,12 @@
-             f.write("\t\t%s;\n" % x)
-     else:
-         f.write("\t\t*;\n")
--    if len(local_abi) > 0:
-+    if abi_match != ["*"]:
-         f.write("\tlocal:\n")
-         for x in local_abi:
-             f.write("\t\t%s;\n" % x[1:])
--    elif abi_match != ["*"]:
--        f.write("\tlocal: *;\n")
-+        if len(global_abi) > 0:
-+            f.write("\t\t*;\n")
-     f.write("};\n")
-     f.close()
- 
diff -Nru ldb-1.1.17/debian/patches/CVE-2015-3223 ldb-1.1.17/debian/patches/CVE-2015-3223
--- ldb-1.1.17/debian/patches/CVE-2015-3223	1970-01-01 00:00:00.000000000 +0000
+++ ldb-1.1.17/debian/patches/CVE-2015-3223	2015-12-17 22:18:52.000000000 +0000
@@ -0,0 +1,82 @@
+From b08459928c4bb7327e02a31cb7361bb2727e8b0d Mon Sep 17 00:00:00 2001
+From: Jeremy Allison <jra@samba.org>
+Date: Tue, 9 Jun 2015 12:42:10 -0700
+Subject: CVE-2015-3223
+
+lib: ldb: Cope with canonicalise_fn returning string "", length 0.
+lib: ldb: Use memmem binary search, not strstr text search.
+
+Values might have embedded zeros.
+
+BUG: https://bugzilla.samba.org/show_bug.cgi?id=11325
+
+Signed-off-by: Jeremy Allison <jra@samba.org>
+Reviewed-by: Ralph Boehme <slow@samba.org>
+
+diff -ur ldb-1.1.17.orig/common/ldb_match.c ldb-1.1.17/common/ldb_match.c
+--- ldb-1.1.17.orig/common/ldb_match.c	2014-06-02 00:22:46.000000000 +0100
++++ ldb-1.1.17/common/ldb_match.c	2015-12-16 01:52:56.509557089 +0000
+@@ -240,7 +240,6 @@
+ 	struct ldb_val val;
+ 	struct ldb_val cnk;
+ 	struct ldb_val *chunk;
+-	char *p, *g;
+ 	uint8_t *save_p = NULL;
+ 	unsigned int c = 0;
+ 
+@@ -270,6 +269,14 @@
+ 		if (cnk.length > val.length) {
+ 			goto mismatch;
+ 		}
++		/*
++		 * Empty strings are returned as length 0. Ensure
++		 * we can cope with this.
++		 */
++		if (cnk.length == 0) {
++			goto mismatch;
++		}
++
+ 		if (memcmp((char *)val.data, (char *)cnk.data, cnk.length) != 0) goto mismatch;
+ 		val.length -= cnk.length;
+ 		val.data += cnk.length;
+@@ -279,20 +286,36 @@
+ 	}
+ 
+ 	while (tree->u.substring.chunks[c]) {
++		uint8_t *p;
+ 
+ 		chunk = tree->u.substring.chunks[c];
+ 		if(a->syntax->canonicalise_fn(ldb, ldb, chunk, &cnk) != 0) goto mismatch;
+ 
+-		/* FIXME: case of embedded nulls */
+-		p = strstr((char *)val.data, (char *)cnk.data);
++		/*
++		 * Empty strings are returned as length 0. Ensure
++		 * we can cope with this.
++		 */
++		if (cnk.length == 0) {
++			goto mismatch;
++		}
++		/*
++		 * Values might be binary blobs. Don't use string
++		 * search, but memory search instead.
++		 */
++		p = memmem((const void *)val.data,val.length,
++			   (const void *)cnk.data, cnk.length);
+ 		if (p == NULL) goto mismatch;
+ 		if ( (! tree->u.substring.chunks[c + 1]) && (! tree->u.substring.end_with_wildcard) ) {
++			uint8_t *g;
+ 			do { /* greedy */
+-				g = strstr((char *)p + cnk.length, (char *)cnk.data);
++				g = memmem(p + cnk.length,
++					val.length - (p - val.data),
++					(const uint8_t *)cnk.data,
++					cnk.length);
+ 				if (g) p = g;
+ 			} while(g);
+ 		}
+-		val.length = val.length - (p - (char *)(val.data)) - cnk.length;
++		val.length = val.length - (p - (uint8_t *)(val.data)) - cnk.length;
+ 		val.data = (uint8_t *)(p + cnk.length);
+ 		c++;
+ 		talloc_free(cnk.data);
diff -Nru ldb-1.1.17/debian/patches/CVE-2015-5330 ldb-1.1.17/debian/patches/CVE-2015-5330
--- ldb-1.1.17/debian/patches/CVE-2015-5330	1970-01-01 00:00:00.000000000 +0000
+++ ldb-1.1.17/debian/patches/CVE-2015-5330	2015-12-17 22:18:52.000000000 +0000
@@ -0,0 +1,229 @@
+From 1a34d6fa5ded464276a196caab9c4c505eb2ee76 Mon Sep 17 00:00:00 2001
+From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
+Date: Tue, 24 Nov 2015 13:07:23 +1300
+Subject: [PATCH 3/9] CVE-2015-5330: ldb_dn: simplify and fix
+ ldb_dn_escape_internal()
+
+Previously we relied on NUL terminated strings and jumped back and
+forth between copying escaped bytes and memcpy()ing un-escaped chunks.
+This simple version is easier to reason about and works with
+unterminated strings. It may also be faster as it avoids reading the
+string twice (first with strcspn, then with memcpy).
+
+Bug: https://bugzilla.samba.org/show_bug.cgi?id=11599
+
+Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
+Pair-programmed-with: Andrew Bartlett <abartlet@samba.org>
+Reviewed-by: Ralph Boehme <slow@samba.org>
+---
+ common/ldb_dn.c | 46 ++++++++++++++++++----------------------------
+ 1 file changed, 18 insertions(+), 28 deletions(-)
+
+diff --git a/common/ldb_dn.c b/common/ldb_dn.c
+index 6b6f90c..1b8e51e 100644
+--- a/common/ldb_dn.c
++++ b/common/ldb_dn.c
+@@ -189,33 +189,23 @@ struct ldb_dn *ldb_dn_new_fmt(TALLOC_CTX *mem_ctx,
+ /* see RFC2253 section 2.4 */
+ static int ldb_dn_escape_internal(char *dst, const char *src, int len)
+ {
+-	const char *p, *s;
++	char c;
+ 	char *d;
+-	size_t l;
+-
+-	p = s = src;
++	int i;
+ 	d = dst;
+ 
+-	while (p - src < len) {
+-		p += strcspn(p, ",=\n\r+<>#;\\\" ");
+-
+-		if (p - src == len) /* found no escapable chars */
+-			break;
+-
+-		/* copy the part of the string before the stop */
+-		memcpy(d, s, p - s);
+-		d += (p - s); /* move to current position */
+-		
+-		switch (*p) {
++	for (i = 0; i < len; i++){
++		c = src[i];
++		switch (c) {
+ 		case ' ':
+-			if (p == src || (p-src)==(len-1)) {
++			if (i == 0 || i == len - 1) {
+ 				/* if at the beginning or end
+ 				 * of the string then escape */
+ 				*d++ = '\\';
+-				*d++ = *p++;					 
++				*d++ = c;
+ 			} else {
+ 				/* otherwise don't escape */
+-				*d++ = *p++;
++				*d++ = c;
+ 			}
+ 			break;
+ 
+@@ -231,30 +221,30 @@ static int ldb_dn_escape_internal(char *dst, const char *src, int len)
+ 		case '?':
+ 			/* these must be escaped using \c form */
+ 			*d++ = '\\';
+-			*d++ = *p++;
++			*d++ = c;
+ 			break;
+ 
+-		default: {
++		case ';':
++		case '\r':
++		case '\n':
++		case '=':
++		case '\0': {
+ 			/* any others get \XX form */
+ 			unsigned char v;
+ 			const char *hexbytes = "0123456789ABCDEF";
+-			v = *(const unsigned char *)p;
++			v = (const unsigned char)c;
+ 			*d++ = '\\';
+ 			*d++ = hexbytes[v>>4];
+ 			*d++ = hexbytes[v&0xF];
+-			p++;
+ 			break;
+ 		}
++		default:
++			*d++ = c;
+ 		}
+-		s = p; /* move forward */
+ 	}
+ 
+-	/* copy the last part (with zero) and return */
+-	l = len - (s - src);
+-	memcpy(d, s, l + 1);
+-
+ 	/* return the length of the resulting string */
+-	return (l + (d - dst));
++	return (d - dst);
+ }
+ 
+ char *ldb_dn_escape_value(TALLOC_CTX *mem_ctx, struct ldb_val value)
+-- 
+2.5.0
+
+
+From cb87cef0ff10de89947fb662147d17de4f34612f Mon Sep 17 00:00:00 2001
+From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
+Date: Tue, 24 Nov 2015 13:09:36 +1300
+Subject: [PATCH 4/9] CVE-2015-5330: ldb_dn_escape_value: use known string
+ length, not strlen()
+
+ldb_dn_escape_internal() reports the number of bytes it copied, so
+lets use that number, rather than using strlen() and hoping a zero got
+in the right place.
+
+Bug: https://bugzilla.samba.org/show_bug.cgi?id=11599
+
+Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
+Pair-programmed-with: Andrew Bartlett <abartlet@samba.org>
+Reviewed-by: Ralph Boehme <slow@samba.org>
+---
+ common/ldb_dn.c | 12 ++++++++----
+ 1 file changed, 8 insertions(+), 4 deletions(-)
+
+diff --git a/common/ldb_dn.c b/common/ldb_dn.c
+index 1b8e51e..a3b8f92 100644
+--- a/common/ldb_dn.c
++++ b/common/ldb_dn.c
+@@ -250,7 +250,7 @@ static int ldb_dn_escape_internal(char *dst, const char *src, int len)
+ char *ldb_dn_escape_value(TALLOC_CTX *mem_ctx, struct ldb_val value)
+ {
+ 	char *dst;
+-
++	size_t len;
+ 	if (!value.length)
+ 		return NULL;
+ 
+@@ -261,10 +261,14 @@ char *ldb_dn_escape_value(TALLOC_CTX *mem_ctx, struct ldb_val value)
+ 		return NULL;
+ 	}
+ 
+-	ldb_dn_escape_internal(dst, (const char *)value.data, value.length);
+-
+-	dst = talloc_realloc(mem_ctx, dst, char, strlen(dst) + 1);
++	len = ldb_dn_escape_internal(dst, (const char *)value.data, value.length);
+ 
++	dst = talloc_realloc(mem_ctx, dst, char, len + 1);
++	if ( ! dst) {
++		talloc_free(dst);
++		return NULL;
++	}
++	dst[len] = '\0';
+ 	return dst;
+ }
+ 
+-- 
+2.5.0
+
+
+From d0b7efef58c29b4a0d5137265a7f43d82ff54d5c Mon Sep 17 00:00:00 2001
+From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
+Date: Thu, 26 Nov 2015 11:17:11 +1300
+Subject: [PATCH 8/9] CVE-2015-5330: ldb_dn_explode: copy strings by length,
+ not terminators
+
+That is, memdup(), not strdup(). The terminators might not be there.
+
+But, we have to make sure we put the terminator on, because we tend to
+assume the terminator is there in other places.
+
+Use talloc_set_name_const() on the resulting chunk so talloc_report()
+remains unchanged.
+
+Bug: https://bugzilla.samba.org/show_bug.cgi?id=11599
+
+Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
+Pair-programmed-with: Andrew Bartlett <abartlet@samba.org>
+Pair-programmed-with: Garming Sam <garming@catalyst.net.nz>
+Pair-programmed-with: Stefan Metzmacher <metze@samba.org>
+Pair-programmed-with: Ralph Boehme <slow@samba.org>
+---
+ common/ldb_dn.c | 9 +++++++--
+ 1 file changed, 7 insertions(+), 2 deletions(-)
+
+diff --git a/common/ldb_dn.c b/common/ldb_dn.c
+index a3b8f92..cd17cda 100644
+--- a/common/ldb_dn.c
++++ b/common/ldb_dn.c
+@@ -586,12 +586,15 @@ static bool ldb_dn_explode(struct ldb_dn *dn)
+ 
+ 				p++;
+ 				*d++ = '\0';
+-				dn->components[dn->comp_num].value.data = (uint8_t *)talloc_strdup(dn->components, dt);
++				dn->components[dn->comp_num].value.data = \
++					(uint8_t *)talloc_memdup(dn->components, dt, l + 1);
+ 				dn->components[dn->comp_num].value.length = l;
+ 				if ( ! dn->components[dn->comp_num].value.data) {
+ 					/* ouch ! */
+ 					goto failed;
+ 				}
++				talloc_set_name_const(dn->components[dn->comp_num].value.data,
++						      (const char *)dn->components[dn->comp_num].value.data);
+ 
+ 				dt = d;
+ 
+@@ -707,11 +710,13 @@ static bool ldb_dn_explode(struct ldb_dn *dn)
+ 	*d++ = '\0';
+ 	dn->components[dn->comp_num].value.length = l;
+ 	dn->components[dn->comp_num].value.data =
+-				(uint8_t *)talloc_strdup(dn->components, dt);
++		(uint8_t *)talloc_memdup(dn->components, dt, l + 1);
+ 	if ( ! dn->components[dn->comp_num].value.data) {
+ 		/* ouch */
+ 		goto failed;
+ 	}
++	talloc_set_name_const(dn->components[dn->comp_num].value.data,
++			      (const char *)dn->components[dn->comp_num].value.data);
+ 
+ 	dn->comp_num++;
+ 
+-- 
+2.5.0
diff -Nru ldb-1.1.17/debian/patches/series ldb-1.1.17/debian/patches/series
--- ldb-1.1.17/debian/patches/series	2015-02-23 18:03:45.000000000 +0000
+++ ldb-1.1.17/debian/patches/series	2015-12-17 22:18:52.000000000 +0000
@@ -0,0 +1,2 @@
+CVE-2015-3223
+CVE-2015-5330

Attachment: signature.asc
Description: PGP signature


Reply to: