Fix CVE-2015-5330

This commit is contained in:
Jakub Hrozek 2015-12-16 12:01:20 +01:00
parent a0366599c4
commit e8212b6212
6 changed files with 359 additions and 1 deletions

66
ldb_binary_cmp.patch Normal file
View File

@ -0,0 +1,66 @@
From e84802e2c61ff06501764496a11ea9e417d70d39 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra@samba.org>
Date: Tue, 9 Jun 2015 14:00:01 -0700
Subject: [PATCH 2/9] CVE-2015-3223: 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>
---
lib/ldb/common/ldb_match.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/lib/ldb/common/ldb_match.c b/lib/ldb/common/ldb_match.c
index 7414289..182c6ce 100644
--- a/lib/ldb/common/ldb_match.c
+++ b/lib/ldb/common/ldb_match.c
@@ -241,7 +241,6 @@ static int ldb_wildcard_compare(struct ldb_context *ldb,
struct ldb_val val;
struct ldb_val cnk;
struct ldb_val *chunk;
- char *p, *g;
uint8_t *save_p = NULL;
unsigned int c = 0;
@@ -288,6 +287,7 @@ static int ldb_wildcard_compare(struct ldb_context *ldb,
}
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;
@@ -299,15 +299,24 @@ static int ldb_wildcard_compare(struct ldb_context *ldb,
if (cnk.length == 0) {
goto mismatch;
}
- p = strstr((char *)val.data, (char *)cnk.data);
+ /*
+ * 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);
--
2.5.0

50
ldb_canon_zero.patch Normal file
View File

@ -0,0 +1,50 @@
From 7d38a883dae528a0216733b4d831f2f20aa9c04b Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra@samba.org>
Date: Tue, 9 Jun 2015 12:42:10 -0700
Subject: [PATCH 1/9] CVE-2015-3223: lib: ldb: Cope with canonicalise_fn
returning string "", length 0.
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>
---
lib/ldb/common/ldb_match.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/lib/ldb/common/ldb_match.c b/lib/ldb/common/ldb_match.c
index a493dae..7414289 100644
--- a/lib/ldb/common/ldb_match.c
+++ b/lib/ldb/common/ldb_match.c
@@ -271,6 +271,14 @@ static int ldb_wildcard_compare(struct ldb_context *ldb,
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;
@@ -284,7 +292,13 @@ static int ldb_wildcard_compare(struct ldb_context *ldb,
chunk = tree->u.substring.chunks[c];
if(a->syntax->canonicalise_fn(ldb, ldb, chunk, &cnk) != 0) goto mismatch;
- /* FIXME: case of embedded nulls */
+ /*
+ * Empty strings are returned as length 0. Ensure
+ * we can cope with this.
+ */
+ if (cnk.length == 0) {
+ goto mismatch;
+ }
p = strstr((char *)val.data, (char *)cnk.data);
if (p == NULL) goto mismatch;
if ( (! tree->u.substring.chunks[c + 1]) && (! tree->u.substring.end_with_wildcard) ) {
--
2.5.0

112
ldb_dn_escape_fix.patch Normal file
View File

@ -0,0 +1,112 @@
From e5b2d30678f80df15a60e6b1101b20e22762aa48 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>
---
lib/ldb/common/ldb_dn.c | 46 ++++++++++++++++++----------------------------
1 file changed, 18 insertions(+), 28 deletions(-)
diff --git a/lib/ldb/common/ldb_dn.c b/lib/ldb/common/ldb_dn.c
index 6b6f90c..1b8e51e 100644
--- a/lib/ldb/common/ldb_dn.c
+++ b/lib/ldb/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

View File

@ -0,0 +1,52 @@
From 85a1c62c942033f6e1b864aeb4fd72c4a8b3d38a 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>
---
lib/ldb/common/ldb_dn.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/lib/ldb/common/ldb_dn.c b/lib/ldb/common/ldb_dn.c
index 1b8e51e..a3b8f92 100644
--- a/lib/ldb/common/ldb_dn.c
+++ b/lib/ldb/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

View File

@ -0,0 +1,65 @@
From bce1b285de7829d9fa110efdfa98fe877ef19e3f 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>
---
lib/ldb/common/ldb_dn.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/lib/ldb/common/ldb_dn.c b/lib/ldb/common/ldb_dn.c
index a3b8f92..cd17cda 100644
--- a/lib/ldb/common/ldb_dn.c
+++ b/lib/ldb/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

View File

@ -9,7 +9,7 @@
Name: libldb Name: libldb
Version: 1.1.23 Version: 1.1.23
Release: 1%{?dist} Release: 2%{?dist}
Group: Development/Libraries Group: Development/Libraries
Summary: A schema-less, ldap like, API and database Summary: A schema-less, ldap like, API and database
Requires: libtalloc%{?_isa} >= %{talloc_version} Requires: libtalloc%{?_isa} >= %{talloc_version}
@ -42,6 +42,11 @@ BuildRequires: openldap-devel
Provides: bundled(libreplace) Provides: bundled(libreplace)
# Patches # Patches
Patch0001: ldb_canon_zero.patch
Patch0002: ldb_binary_cmp.patch
Patch0003: ldb_dn_escape_fix.patch
Patch0004: ldb_dn_escape_no_strlen.patch
Patch0005: ldb_dn_explode_no_strlen.patch
%description %description
An extensible library that implements an LDAP like API to access remote LDAP An extensible library that implements an LDAP like API to access remote LDAP
@ -86,6 +91,11 @@ Development files for the Python bindings for the LDB library
%prep %prep
%setup -q -n ldb-%{version} %setup -q -n ldb-%{version}
%patch0001 -p3 -b .ldb_canon_zero
%patch0002 -p3 -b .ldb_binary_cmp
%patch0003 -p3 -b .ldb_dn_escape_fix
%patch0004 -p3 -b .ldb_dn_escape_no_strlen
%patch0005 -p3 -b .ldb_dn_explode_no_strlen
%build %build
@ -178,6 +188,9 @@ rm -rf %{buildroot}
%postun -n pyldb -p /sbin/ldconfig %postun -n pyldb -p /sbin/ldconfig
%changelog %changelog
* Wed Dec 16 2015 Jakub Hrozek <jhrozek@redhat.com> - 1.1.23-2
- Fix CVE-2015-5330
* Thu Nov 12 2015 Jakub Hrozek <jhrozek@redhat.com> - 1.1.23-1 * Thu Nov 12 2015 Jakub Hrozek <jhrozek@redhat.com> - 1.1.23-1
- New upstream release 1.1.23 - New upstream release 1.1.23