From fb456954f332c07a645226d59b3b00ec252f8b26 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 9 Jun 2015 12:42:10 -0700 Subject: [PATCH 01/17] 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 Reviewed-by: Ralph Boehme --- 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 7918aec..8bdb0e1 100644 --- a/lib/ldb/common/ldb_match.c +++ b/lib/ldb/common/ldb_match.c @@ -270,6 +270,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; @@ -283,7 +291,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) ) { -- 1.9.1 From bb1b783ee9d7259cfc6a1fe882f22189747f8684 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 9 Jun 2015 14:00:01 -0700 Subject: [PATCH 02/17] 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 Reviewed-by: Ralph Boehme --- 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 8bdb0e1..0f5c5b5 100644 --- a/lib/ldb/common/ldb_match.c +++ b/lib/ldb/common/ldb_match.c @@ -240,7 +240,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; @@ -287,6 +286,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; @@ -298,15 +298,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); -- 1.9.1 From 1aef718f3cc175d90d40202a333042a38ba382b1 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Tue, 24 Nov 2015 13:07:23 +1300 Subject: [PATCH 03/17] 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 Pair-programmed-with: Andrew Bartlett Reviewed-by: Ralph Boehme --- 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) -- 1.9.1 From 7bcac237656083e67bbac9b50be9b319bb2d7eb8 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Tue, 24 Nov 2015 13:09:36 +1300 Subject: [PATCH 04/17] 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 Pair-programmed-with: Andrew Bartlett Reviewed-by: Ralph Boehme --- 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; } -- 1.9.1 From 5f3c7541c2f10ac2174538288f6569af587d69f0 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Tue, 24 Nov 2015 13:47:16 +1300 Subject: [PATCH 05/17] CVE-2015-5330: Fix handling of unicode near string endings Until now next_codepoint_ext() and next_codepoint_handle_ext() were using strnlen(str, 5) to determine how much string they should try to decode. This ended up looking past the end of the string when it was not null terminated and the final character looked like a multi-byte encoding. The fix is to let the caller say how long the string can be. Bug: https://bugzilla.samba.org/show_bug.cgi?id=11599 Signed-off-by: Douglas Bagnall Pair-programmed-with: Andrew Bartlett Reviewed-by: Ralph Boehme --- lib/util/charset/charset.h | 9 +++++---- lib/util/charset/codepoints.c | 24 ++++++++++++++++-------- lib/util/charset/util_str.c | 3 ++- lib/util/charset/util_unistr.c | 3 ++- 4 files changed, 25 insertions(+), 14 deletions(-) diff --git a/lib/util/charset/charset.h b/lib/util/charset/charset.h index e4297e4..060f1cf 100644 --- a/lib/util/charset/charset.h +++ b/lib/util/charset/charset.h @@ -171,15 +171,16 @@ smb_iconv_t get_conv_handle(struct smb_iconv_handle *ic, charset_t from, charset_t to); const char *charset_name(struct smb_iconv_handle *ic, charset_t ch); -codepoint_t next_codepoint_ext(const char *str, charset_t src_charset, - size_t *size); +codepoint_t next_codepoint_ext(const char *str, size_t len, + charset_t src_charset, size_t *size); codepoint_t next_codepoint(const char *str, size_t *size); ssize_t push_codepoint(char *str, codepoint_t c); /* codepoints */ codepoint_t next_codepoint_handle_ext(struct smb_iconv_handle *ic, - const char *str, charset_t src_charset, - size_t *size); + const char *str, size_t len, + charset_t src_charset, + size_t *size); codepoint_t next_codepoint_handle(struct smb_iconv_handle *ic, const char *str, size_t *size); ssize_t push_codepoint_handle(struct smb_iconv_handle *ic, diff --git a/lib/util/charset/codepoints.c b/lib/util/charset/codepoints.c index 0984164..542eeae 100644 --- a/lib/util/charset/codepoints.c +++ b/lib/util/charset/codepoints.c @@ -319,7 +319,8 @@ smb_iconv_t get_conv_handle(struct smb_iconv_handle *ic, */ _PUBLIC_ codepoint_t next_codepoint_handle_ext( struct smb_iconv_handle *ic, - const char *str, charset_t src_charset, + const char *str, size_t len, + charset_t src_charset, size_t *bytes_consumed) { /* it cannot occupy more than 4 bytes in UTF16 format */ @@ -339,7 +340,7 @@ _PUBLIC_ codepoint_t next_codepoint_handle_ext( * we assume that no multi-byte character can take more than 5 bytes. * This is OK as we only support codepoints up to 1M (U+100000) */ - ilen_orig = strnlen(str, 5); + ilen_orig = MIN(len, 5); ilen = ilen_orig; descriptor = get_conv_handle(ic, src_charset, CH_UTF16); @@ -395,9 +396,16 @@ _PUBLIC_ codepoint_t next_codepoint_handle_ext( return INVALID_CODEPOINT if the next character cannot be converted */ _PUBLIC_ codepoint_t next_codepoint_handle(struct smb_iconv_handle *ic, - const char *str, size_t *size) + const char *str, size_t *size) { - return next_codepoint_handle_ext(ic, str, CH_UNIX, size); + /* + * We assume that no multi-byte character can take more than 5 bytes + * thus avoiding walking all the way down a long string. This is OK as + * Unicode codepoints only go up to (U+10ffff), which can always be + * encoded in 4 bytes or less. + */ + return next_codepoint_handle_ext(ic, str, strnlen(str, 5), CH_UNIX, + size); } /* @@ -459,11 +467,11 @@ _PUBLIC_ ssize_t push_codepoint_handle(struct smb_iconv_handle *ic, return 5 - olen; } -_PUBLIC_ codepoint_t next_codepoint_ext(const char *str, charset_t src_charset, - size_t *size) +_PUBLIC_ codepoint_t next_codepoint_ext(const char *str, size_t len, + charset_t src_charset, size_t *size) { - return next_codepoint_handle_ext(get_iconv_handle(), str, - src_charset, size); + return next_codepoint_handle_ext(get_iconv_handle(), str, len, + src_charset, size); } _PUBLIC_ codepoint_t next_codepoint(const char *str, size_t *size) diff --git a/lib/util/charset/util_str.c b/lib/util/charset/util_str.c index d2e6cbb..2653bfc 100644 --- a/lib/util/charset/util_str.c +++ b/lib/util/charset/util_str.c @@ -210,7 +210,8 @@ _PUBLIC_ size_t strlen_m_ext_handle(struct smb_iconv_handle *ic, while (*s) { size_t c_size; - codepoint_t c = next_codepoint_handle_ext(ic, s, src_charset, &c_size); + codepoint_t c = next_codepoint_handle_ext(ic, s, strnlen(s, 5), + src_charset, &c_size); s += c_size; switch (dst_charset) { diff --git a/lib/util/charset/util_unistr.c b/lib/util/charset/util_unistr.c index e4ae650..f299269 100644 --- a/lib/util/charset/util_unistr.c +++ b/lib/util/charset/util_unistr.c @@ -112,7 +112,8 @@ _PUBLIC_ char *strupper_talloc_n_handle(struct smb_iconv_handle *iconv_handle, while (n-- && *src) { size_t c_size; - codepoint_t c = next_codepoint_handle(iconv_handle, src, &c_size); + codepoint_t c = next_codepoint_handle_ext(iconv_handle, src, n, + CH_UNIX, &c_size); src += c_size; c = toupper_m(c); -- 1.9.1 From a561ae6294fa926bf3a15b9aaf3d18d25d5e971f Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Tue, 24 Nov 2015 13:49:09 +1300 Subject: [PATCH 06/17] CVE-2015-5330: strupper_talloc_n_handle(): properly count characters When a codepoint eats more than one byte we really want to know, especially if the string is not NUL terminated. Bug: https://bugzilla.samba.org/show_bug.cgi?id=11599 Signed-off-by: Douglas Bagnall Pair-programmed-with: Andrew Bartlett Reviewed-by: Ralph Boehme --- lib/util/charset/util_unistr.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/util/charset/util_unistr.c b/lib/util/charset/util_unistr.c index f299269..2cc8718 100644 --- a/lib/util/charset/util_unistr.c +++ b/lib/util/charset/util_unistr.c @@ -110,11 +110,12 @@ _PUBLIC_ char *strupper_talloc_n_handle(struct smb_iconv_handle *iconv_handle, return NULL; } - while (n-- && *src) { + while (n && *src) { size_t c_size; codepoint_t c = next_codepoint_handle_ext(iconv_handle, src, n, CH_UNIX, &c_size); src += c_size; + n -= c_size; c = toupper_m(c); -- 1.9.1 From f07626d0297ed6bd21623409e1ea1ae1138d23a8 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Tue, 24 Nov 2015 13:54:09 +1300 Subject: [PATCH 07/17] CVE-2015-5330: next_codepoint_handle_ext: don't short-circuit UTF16 low bytes UTF16 contains zero bytes when it is encoding ASCII (for example), so we can't assume the absense of the 0x80 bit means a one byte encoding. No current callers use UTF16. Bug: https://bugzilla.samba.org/show_bug.cgi?id=11599 Signed-off-by: Douglas Bagnall Pair-programmed-with: Andrew Bartlett Reviewed-by: Ralph Boehme --- lib/util/charset/codepoints.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/util/charset/codepoints.c b/lib/util/charset/codepoints.c index 542eeae..19d084f 100644 --- a/lib/util/charset/codepoints.c +++ b/lib/util/charset/codepoints.c @@ -331,7 +331,10 @@ _PUBLIC_ codepoint_t next_codepoint_handle_ext( size_t olen; char *outbuf; - if ((str[0] & 0x80) == 0) { + + if (((str[0] & 0x80) == 0) && (src_charset == CH_DOS || + src_charset == CH_UNIX || + src_charset == CH_UTF8)) { *bytes_consumed = 1; return (codepoint_t)str[0]; } -- 1.9.1 From 83f1d39cd9ab9b8b548602f9ee806a994fca9d0c Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Thu, 26 Nov 2015 11:17:11 +1300 Subject: [PATCH 08/17] 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 Pair-programmed-with: Andrew Bartlett Pair-programmed-with: Garming Sam Pair-programmed-with: Stefan Metzmacher Pair-programmed-with: Ralph Boehme --- 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++; -- 1.9.1 From 582d0e7b7549008c908cb30878a1db0bbe4d21bb Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Tue, 8 Dec 2015 10:55:42 +0100 Subject: [PATCH 09/17] ldb: bump version of the required system ldb to 1.1.24 This is needed to ensure we build against a system ldb library that contains the fixes for CVE-2015-5330 and CVE-2015-3223. autobuild must still be able to build against the older version 1.1.17 including the patches. Bug: https://bugzilla.samba.org/show_bug.cgi?id=11325 Bug: https://bugzilla.samba.org/show_bug.cgi?id=11599 Bug: https://bugzilla.samba.org/show_bug.cgi?id=11636 Signed-off-by: Ralph Boehme Reviewed-by: Stefan Metzmacher --- lib/ldb/wscript | 5 +++-- script/autobuild.py | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/ldb/wscript b/lib/ldb/wscript index fb32ecd..794d6db 100755 --- a/lib/ldb/wscript +++ b/lib/ldb/wscript @@ -2,6 +2,7 @@ APPNAME = 'ldb' VERSION = '1.1.17' +SYSTEM_VERSION = '1.1.24' blddir = 'bin' @@ -46,11 +47,11 @@ def configure(conf): conf.env.standalone_ldb = conf.IN_LAUNCH_DIR() if not conf.env.standalone_ldb: - if conf.CHECK_BUNDLED_SYSTEM_PKG('ldb', minversion=VERSION, + if conf.CHECK_BUNDLED_SYSTEM_PKG('ldb', minversion=SYSTEM_VERSION, onlyif='talloc tdb tevent', implied_deps='replace talloc tdb tevent'): conf.define('USING_SYSTEM_LDB', 1) - if conf.CHECK_BUNDLED_SYSTEM_PKG('pyldb-util', minversion=VERSION, + if conf.CHECK_BUNDLED_SYSTEM_PKG('pyldb-util', minversion=SYSTEM_VERSION, onlyif='talloc tdb tevent ldb', implied_deps='replace talloc tdb tevent ldb'): conf.define('USING_SYSTEM_PYLDB_UTIL', 1) diff --git a/script/autobuild.py b/script/autobuild.py index 9d117c6..192061a 100755 --- a/script/autobuild.py +++ b/script/autobuild.py @@ -82,7 +82,7 @@ tasks = { ("ldb-make", "cd lib/ldb && make", "text/plain"), ("ldb-install", "cd lib/ldb && make install", "text/plain"), - ("configure", "PYTHONPATH=${PYTHON_PREFIX}/site-packages:$PYTHONPATH PKG_CONFIG_PATH=$PKG_CONFIG_PATH:${PREFIX_DIR}/lib/pkgconfig ./configure --bundled-libraries=!talloc,!tdb,!pytdb,!ntdb,!pyntdb,!ldb,!pyldb,!tevent,!pytevent --abi-check --enable-debug -C ${PREFIX}", "text/plain"), + ("configure", "PYTHONPATH=${PYTHON_PREFIX}/site-packages:$PYTHONPATH PKG_CONFIG_PATH=$PKG_CONFIG_PATH:${PREFIX_DIR}/lib/pkgconfig ./configure --bundled-libraries=!talloc,!tdb,!pytdb,!ntdb,!pyntdb,!ldb,!pyldb,!tevent,!pytevent --minimum-library-version=ldb:1.1.17,pyldb-util:1.1.17 --abi-check --enable-debug -C ${PREFIX}", "text/plain"), ("make", "make", "text/plain"), ("install", "make install", "text/plain")], -- 1.9.1 From 530d50a1abdcdf4d1775652d4c456c1274d83d8d Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Mon, 22 Sep 2014 16:08:26 -0700 Subject: [PATCH 10/17] CVE-2015-7540: s4: libcli: ldap message - Ensure all asn1_XX returns are checked. BUG: https://bugzilla.samba.org/show_bug.cgi?id=9187 Signed-off-by: Jeremy Allison Reviewed-by: Ronnie Sahlberg Autobuild-User(master): Jeremy Allison Autobuild-Date(master): Fri Sep 26 03:15:00 CEST 2014 on sn-devel-104 (cherry picked from commit 69a7e3cfdc8dbba9c8dcfdfae82d2894c7247e15) --- libcli/ldap/ldap_message.c | 786 ++++++++++++++++++------------------ libcli/ldap/ldap_message.h | 2 +- source4/libcli/ldap/ldap_controls.c | 8 +- 3 files changed, 401 insertions(+), 395 deletions(-) diff --git a/libcli/ldap/ldap_message.c b/libcli/ldap/ldap_message.c index 1c5542c..ba94f4c 100644 --- a/libcli/ldap/ldap_message.c +++ b/libcli/ldap/ldap_message.c @@ -229,31 +229,31 @@ static bool ldap_push_filter(struct asn1_data *data, struct ldb_parse_tree *tree switch (tree->operation) { case LDB_OP_AND: case LDB_OP_OR: - asn1_push_tag(data, ASN1_CONTEXT(tree->operation==LDB_OP_AND?0:1)); + if (!asn1_push_tag(data, ASN1_CONTEXT(tree->operation==LDB_OP_AND?0:1))) return false; for (i=0; iu.list.num_elements; i++) { if (!ldap_push_filter(data, tree->u.list.elements[i])) { return false; } } - asn1_pop_tag(data); + if (!asn1_pop_tag(data)) return false; break; case LDB_OP_NOT: - asn1_push_tag(data, ASN1_CONTEXT(2)); + if (!asn1_push_tag(data, ASN1_CONTEXT(2))) return false; if (!ldap_push_filter(data, tree->u.isnot.child)) { return false; } - asn1_pop_tag(data); + if (!asn1_pop_tag(data)) return false; break; case LDB_OP_EQUALITY: /* equality test */ - asn1_push_tag(data, ASN1_CONTEXT(3)); - asn1_write_OctetString(data, tree->u.equality.attr, - strlen(tree->u.equality.attr)); - asn1_write_OctetString(data, tree->u.equality.value.data, - tree->u.equality.value.length); - asn1_pop_tag(data); + if (!asn1_push_tag(data, ASN1_CONTEXT(3))) return false; + if (!asn1_write_OctetString(data, tree->u.equality.attr, + strlen(tree->u.equality.attr))) return false; + if (!asn1_write_OctetString(data, tree->u.equality.value.data, + tree->u.equality.value.length)) return false; + if (!asn1_pop_tag(data)) return false; break; case LDB_OP_SUBSTRING: @@ -266,16 +266,16 @@ static bool ldap_push_filter(struct asn1_data *data, struct ldb_parse_tree *tree any [1] LDAPString, final [2] LDAPString } } */ - asn1_push_tag(data, ASN1_CONTEXT(4)); - asn1_write_OctetString(data, tree->u.substring.attr, strlen(tree->u.substring.attr)); - asn1_push_tag(data, ASN1_SEQUENCE(0)); + if (!asn1_push_tag(data, ASN1_CONTEXT(4))) return false; + if (!asn1_write_OctetString(data, tree->u.substring.attr, strlen(tree->u.substring.attr))) return false; + if (!asn1_push_tag(data, ASN1_SEQUENCE(0))) return false; if (tree->u.substring.chunks && tree->u.substring.chunks[0]) { i = 0; if (!tree->u.substring.start_with_wildcard) { - asn1_push_tag(data, ASN1_CONTEXT_SIMPLE(0)); - asn1_write_DATA_BLOB_LDAPString(data, tree->u.substring.chunks[i]); - asn1_pop_tag(data); + if (!asn1_push_tag(data, ASN1_CONTEXT_SIMPLE(0))) return false; + if (!asn1_write_DATA_BLOB_LDAPString(data, tree->u.substring.chunks[i])) return false; + if (!asn1_pop_tag(data)) return false; i++; } while (tree->u.substring.chunks[i]) { @@ -287,51 +287,51 @@ static bool ldap_push_filter(struct asn1_data *data, struct ldb_parse_tree *tree } else { ctx = 1; } - asn1_push_tag(data, ASN1_CONTEXT_SIMPLE(ctx)); - asn1_write_DATA_BLOB_LDAPString(data, tree->u.substring.chunks[i]); - asn1_pop_tag(data); + if (!asn1_push_tag(data, ASN1_CONTEXT_SIMPLE(ctx))) return false; + if (!asn1_write_DATA_BLOB_LDAPString(data, tree->u.substring.chunks[i])) return false; + if (!asn1_pop_tag(data)) return false; i++; } } - asn1_pop_tag(data); - asn1_pop_tag(data); + if (!asn1_pop_tag(data)) return false; + if (!asn1_pop_tag(data)) return false; break; case LDB_OP_GREATER: /* greaterOrEqual test */ - asn1_push_tag(data, ASN1_CONTEXT(5)); - asn1_write_OctetString(data, tree->u.comparison.attr, - strlen(tree->u.comparison.attr)); - asn1_write_OctetString(data, tree->u.comparison.value.data, - tree->u.comparison.value.length); - asn1_pop_tag(data); + if (!asn1_push_tag(data, ASN1_CONTEXT(5))) return false; + if (!asn1_write_OctetString(data, tree->u.comparison.attr, + strlen(tree->u.comparison.attr))) return false; + if (!asn1_write_OctetString(data, tree->u.comparison.value.data, + tree->u.comparison.value.length)) return false; + if (!asn1_pop_tag(data)) return false; break; case LDB_OP_LESS: /* lessOrEqual test */ - asn1_push_tag(data, ASN1_CONTEXT(6)); - asn1_write_OctetString(data, tree->u.comparison.attr, - strlen(tree->u.comparison.attr)); - asn1_write_OctetString(data, tree->u.comparison.value.data, - tree->u.comparison.value.length); - asn1_pop_tag(data); + if (!asn1_push_tag(data, ASN1_CONTEXT(6))) return false; + if (!asn1_write_OctetString(data, tree->u.comparison.attr, + strlen(tree->u.comparison.attr))) return false; + if (!asn1_write_OctetString(data, tree->u.comparison.value.data, + tree->u.comparison.value.length)) return false; + if (!asn1_pop_tag(data)) return false; break; case LDB_OP_PRESENT: /* present test */ - asn1_push_tag(data, ASN1_CONTEXT_SIMPLE(7)); - asn1_write_LDAPString(data, tree->u.present.attr); - asn1_pop_tag(data); + if (!asn1_push_tag(data, ASN1_CONTEXT_SIMPLE(7))) return false; + if (!asn1_write_LDAPString(data, tree->u.present.attr)) return false; + if (!asn1_pop_tag(data)) return false; return !data->has_error; case LDB_OP_APPROX: /* approx test */ - asn1_push_tag(data, ASN1_CONTEXT(8)); - asn1_write_OctetString(data, tree->u.comparison.attr, - strlen(tree->u.comparison.attr)); - asn1_write_OctetString(data, tree->u.comparison.value.data, - tree->u.comparison.value.length); - asn1_pop_tag(data); + if (!asn1_push_tag(data, ASN1_CONTEXT(8))) return false; + if (!asn1_write_OctetString(data, tree->u.comparison.attr, + strlen(tree->u.comparison.attr))) return false; + if (!asn1_write_OctetString(data, tree->u.comparison.value.data, + tree->u.comparison.value.length)) return false; + if (!asn1_pop_tag(data)) return false; break; case LDB_OP_EXTENDED: @@ -343,24 +343,24 @@ static bool ldap_push_filter(struct asn1_data *data, struct ldb_parse_tree *tree dnAttributes [4] BOOLEAN DEFAULT FALSE } */ - asn1_push_tag(data, ASN1_CONTEXT(9)); + if (!asn1_push_tag(data, ASN1_CONTEXT(9))) return false; if (tree->u.extended.rule_id) { - asn1_push_tag(data, ASN1_CONTEXT_SIMPLE(1)); - asn1_write_LDAPString(data, tree->u.extended.rule_id); - asn1_pop_tag(data); + if (!asn1_push_tag(data, ASN1_CONTEXT_SIMPLE(1))) return false; + if (!asn1_write_LDAPString(data, tree->u.extended.rule_id)) return false; + if (!asn1_pop_tag(data)) return false; } if (tree->u.extended.attr) { - asn1_push_tag(data, ASN1_CONTEXT_SIMPLE(2)); - asn1_write_LDAPString(data, tree->u.extended.attr); - asn1_pop_tag(data); + if (!asn1_push_tag(data, ASN1_CONTEXT_SIMPLE(2))) return false; + if (!asn1_write_LDAPString(data, tree->u.extended.attr)) return false; + if (!asn1_pop_tag(data)) return false; } - asn1_push_tag(data, ASN1_CONTEXT_SIMPLE(3)); - asn1_write_DATA_BLOB_LDAPString(data, &tree->u.extended.value); - asn1_pop_tag(data); - asn1_push_tag(data, ASN1_CONTEXT_SIMPLE(4)); - asn1_write_uint8(data, tree->u.extended.dnAttributes); - asn1_pop_tag(data); - asn1_pop_tag(data); + if (!asn1_push_tag(data, ASN1_CONTEXT_SIMPLE(3))) return false; + if (!asn1_write_DATA_BLOB_LDAPString(data, &tree->u.extended.value)) return false; + if (!asn1_pop_tag(data)) return false; + if (!asn1_push_tag(data, ASN1_CONTEXT_SIMPLE(4))) return false; + if (!asn1_write_uint8(data, tree->u.extended.dnAttributes)) return false; + if (!asn1_pop_tag(data)) return false; + if (!asn1_pop_tag(data)) return false; break; default: @@ -369,20 +369,21 @@ static bool ldap_push_filter(struct asn1_data *data, struct ldb_parse_tree *tree return !data->has_error; } -static void ldap_encode_response(struct asn1_data *data, struct ldap_Result *result) +static bool ldap_encode_response(struct asn1_data *data, struct ldap_Result *result) { - asn1_write_enumerated(data, result->resultcode); - asn1_write_OctetString(data, result->dn, - (result->dn) ? strlen(result->dn) : 0); - asn1_write_OctetString(data, result->errormessage, + if (!asn1_write_enumerated(data, result->resultcode)) return false; + if (!asn1_write_OctetString(data, result->dn, + (result->dn) ? strlen(result->dn) : 0)) return false; + if (!asn1_write_OctetString(data, result->errormessage, (result->errormessage) ? - strlen(result->errormessage) : 0); + strlen(result->errormessage) : 0)) return false; if (result->referral) { - asn1_push_tag(data, ASN1_CONTEXT(3)); - asn1_write_OctetString(data, result->referral, - strlen(result->referral)); - asn1_pop_tag(data); + if (!asn1_push_tag(data, ASN1_CONTEXT(3))) return false; + if (!asn1_write_OctetString(data, result->referral, + strlen(result->referral))) return false; + if (!asn1_pop_tag(data)) return false; } + return true; } _PUBLIC_ bool ldap_encode(struct ldap_message *msg, @@ -394,286 +395,286 @@ _PUBLIC_ bool ldap_encode(struct ldap_message *msg, if (!data) return false; - asn1_push_tag(data, ASN1_SEQUENCE(0)); - asn1_write_Integer(data, msg->messageid); + if (!asn1_push_tag(data, ASN1_SEQUENCE(0))) goto err; + if (!asn1_write_Integer(data, msg->messageid)) goto err; switch (msg->type) { case LDAP_TAG_BindRequest: { struct ldap_BindRequest *r = &msg->r.BindRequest; - asn1_push_tag(data, ASN1_APPLICATION(msg->type)); - asn1_write_Integer(data, r->version); - asn1_write_OctetString(data, r->dn, - (r->dn != NULL) ? strlen(r->dn) : 0); + if (!asn1_push_tag(data, ASN1_APPLICATION(msg->type))) goto err; + if (!asn1_write_Integer(data, r->version)) goto err; + if (!asn1_write_OctetString(data, r->dn, + (r->dn != NULL) ? strlen(r->dn) : 0)) goto err; switch (r->mechanism) { case LDAP_AUTH_MECH_SIMPLE: /* context, primitive */ - asn1_push_tag(data, ASN1_CONTEXT_SIMPLE(0)); - asn1_write(data, r->creds.password, - strlen(r->creds.password)); - asn1_pop_tag(data); + if (!asn1_push_tag(data, ASN1_CONTEXT_SIMPLE(0))) goto err; + if (!asn1_write(data, r->creds.password, + strlen(r->creds.password))) goto err; + if (!asn1_pop_tag(data)) goto err; break; case LDAP_AUTH_MECH_SASL: /* context, constructed */ - asn1_push_tag(data, ASN1_CONTEXT(3)); - asn1_write_OctetString(data, r->creds.SASL.mechanism, - strlen(r->creds.SASL.mechanism)); + if (!asn1_push_tag(data, ASN1_CONTEXT(3))) goto err; + if (!asn1_write_OctetString(data, r->creds.SASL.mechanism, + strlen(r->creds.SASL.mechanism))) goto err; if (r->creds.SASL.secblob) { - asn1_write_OctetString(data, r->creds.SASL.secblob->data, - r->creds.SASL.secblob->length); + if (!asn1_write_OctetString(data, r->creds.SASL.secblob->data, + r->creds.SASL.secblob->length)) goto err; } - asn1_pop_tag(data); + if (!asn1_pop_tag(data)) goto err; break; default: - return false; + goto err; } - asn1_pop_tag(data); + if (!asn1_pop_tag(data)) goto err; break; } case LDAP_TAG_BindResponse: { struct ldap_BindResponse *r = &msg->r.BindResponse; - asn1_push_tag(data, ASN1_APPLICATION(msg->type)); - ldap_encode_response(data, &r->response); + if (!asn1_push_tag(data, ASN1_APPLICATION(msg->type))) goto err; + if (!ldap_encode_response(data, &r->response)) goto err; if (r->SASL.secblob) { - asn1_write_ContextSimple(data, 7, r->SASL.secblob); + if (!asn1_write_ContextSimple(data, 7, r->SASL.secblob)) goto err; } - asn1_pop_tag(data); + if (!asn1_pop_tag(data)) goto err; break; } case LDAP_TAG_UnbindRequest: { /* struct ldap_UnbindRequest *r = &msg->r.UnbindRequest; */ - asn1_push_tag(data, ASN1_APPLICATION_SIMPLE(msg->type)); - asn1_pop_tag(data); + if (!asn1_push_tag(data, ASN1_APPLICATION_SIMPLE(msg->type))) goto err; + if (!asn1_pop_tag(data)) goto err; break; } case LDAP_TAG_SearchRequest: { struct ldap_SearchRequest *r = &msg->r.SearchRequest; - asn1_push_tag(data, ASN1_APPLICATION(msg->type)); - asn1_write_OctetString(data, r->basedn, strlen(r->basedn)); - asn1_write_enumerated(data, r->scope); - asn1_write_enumerated(data, r->deref); - asn1_write_Integer(data, r->sizelimit); - asn1_write_Integer(data, r->timelimit); - asn1_write_BOOLEAN(data, r->attributesonly); + if (!asn1_push_tag(data, ASN1_APPLICATION(msg->type))) goto err; + if (!asn1_write_OctetString(data, r->basedn, strlen(r->basedn))) goto err; + if (!asn1_write_enumerated(data, r->scope)) goto err; + if (!asn1_write_enumerated(data, r->deref)) goto err; + if (!asn1_write_Integer(data, r->sizelimit)) goto err; + if (!asn1_write_Integer(data, r->timelimit)) goto err; + if (!asn1_write_BOOLEAN(data, r->attributesonly)) goto err; if (!ldap_push_filter(data, r->tree)) { - return false; + goto err; } - asn1_push_tag(data, ASN1_SEQUENCE(0)); + if (!asn1_push_tag(data, ASN1_SEQUENCE(0))) goto err; for (i=0; inum_attributes; i++) { - asn1_write_OctetString(data, r->attributes[i], - strlen(r->attributes[i])); + if (!asn1_write_OctetString(data, r->attributes[i], + strlen(r->attributes[i]))) goto err; } - asn1_pop_tag(data); - asn1_pop_tag(data); + if (!asn1_pop_tag(data)) goto err; + if (!asn1_pop_tag(data)) goto err; break; } case LDAP_TAG_SearchResultEntry: { struct ldap_SearchResEntry *r = &msg->r.SearchResultEntry; - asn1_push_tag(data, ASN1_APPLICATION(msg->type)); - asn1_write_OctetString(data, r->dn, strlen(r->dn)); - asn1_push_tag(data, ASN1_SEQUENCE(0)); + if (!asn1_push_tag(data, ASN1_APPLICATION(msg->type))) goto err; + if (!asn1_write_OctetString(data, r->dn, strlen(r->dn))) goto err; + if (!asn1_push_tag(data, ASN1_SEQUENCE(0))) goto err; for (i=0; inum_attributes; i++) { struct ldb_message_element *attr = &r->attributes[i]; - asn1_push_tag(data, ASN1_SEQUENCE(0)); - asn1_write_OctetString(data, attr->name, - strlen(attr->name)); - asn1_push_tag(data, ASN1_SEQUENCE(1)); + if (!asn1_push_tag(data, ASN1_SEQUENCE(0))) goto err; + if (!asn1_write_OctetString(data, attr->name, + strlen(attr->name))) goto err; + if (!asn1_push_tag(data, ASN1_SEQUENCE(1))) goto err; for (j=0; jnum_values; j++) { - asn1_write_OctetString(data, + if (!asn1_write_OctetString(data, attr->values[j].data, - attr->values[j].length); + attr->values[j].length)) goto err; } - asn1_pop_tag(data); - asn1_pop_tag(data); + if (!asn1_pop_tag(data)) goto err; + if (!asn1_pop_tag(data)) goto err; } - asn1_pop_tag(data); - asn1_pop_tag(data); + if (!asn1_pop_tag(data)) goto err; + if (!asn1_pop_tag(data)) goto err; break; } case LDAP_TAG_SearchResultDone: { struct ldap_Result *r = &msg->r.SearchResultDone; - asn1_push_tag(data, ASN1_APPLICATION(msg->type)); - ldap_encode_response(data, r); - asn1_pop_tag(data); + if (!asn1_push_tag(data, ASN1_APPLICATION(msg->type))) goto err; + if (!ldap_encode_response(data, r)) goto err; + if (!asn1_pop_tag(data)) goto err; break; } case LDAP_TAG_ModifyRequest: { struct ldap_ModifyRequest *r = &msg->r.ModifyRequest; - asn1_push_tag(data, ASN1_APPLICATION(msg->type)); - asn1_write_OctetString(data, r->dn, strlen(r->dn)); - asn1_push_tag(data, ASN1_SEQUENCE(0)); + if (!asn1_push_tag(data, ASN1_APPLICATION(msg->type))) goto err; + if (!asn1_write_OctetString(data, r->dn, strlen(r->dn))) goto err; + if (!asn1_push_tag(data, ASN1_SEQUENCE(0))) goto err; for (i=0; inum_mods; i++) { struct ldb_message_element *attrib = &r->mods[i].attrib; - asn1_push_tag(data, ASN1_SEQUENCE(0)); - asn1_write_enumerated(data, r->mods[i].type); - asn1_push_tag(data, ASN1_SEQUENCE(0)); - asn1_write_OctetString(data, attrib->name, - strlen(attrib->name)); - asn1_push_tag(data, ASN1_SET); + if (!asn1_push_tag(data, ASN1_SEQUENCE(0))) goto err; + if (!asn1_write_enumerated(data, r->mods[i].type)) goto err; + if (!asn1_push_tag(data, ASN1_SEQUENCE(0))) goto err; + if (!asn1_write_OctetString(data, attrib->name, + strlen(attrib->name))) goto err; + if (!asn1_push_tag(data, ASN1_SET)) goto err; for (j=0; jnum_values; j++) { - asn1_write_OctetString(data, + if (!asn1_write_OctetString(data, attrib->values[j].data, - attrib->values[j].length); + attrib->values[j].length)) goto err; } - asn1_pop_tag(data); - asn1_pop_tag(data); - asn1_pop_tag(data); + if (!asn1_pop_tag(data)) goto err; + if (!asn1_pop_tag(data)) goto err; + if (!asn1_pop_tag(data)) goto err; } - asn1_pop_tag(data); - asn1_pop_tag(data); + if (!asn1_pop_tag(data)) goto err; + if (!asn1_pop_tag(data)) goto err; break; } case LDAP_TAG_ModifyResponse: { struct ldap_Result *r = &msg->r.ModifyResponse; - asn1_push_tag(data, ASN1_APPLICATION(msg->type)); - ldap_encode_response(data, r); - asn1_pop_tag(data); + if (!asn1_push_tag(data, ASN1_APPLICATION(msg->type))) goto err; + if (!ldap_encode_response(data, r)) goto err; + if (!asn1_pop_tag(data)) goto err; break; } case LDAP_TAG_AddRequest: { struct ldap_AddRequest *r = &msg->r.AddRequest; - asn1_push_tag(data, ASN1_APPLICATION(msg->type)); - asn1_write_OctetString(data, r->dn, strlen(r->dn)); - asn1_push_tag(data, ASN1_SEQUENCE(0)); + if (!asn1_push_tag(data, ASN1_APPLICATION(msg->type))) goto err; + if (!asn1_write_OctetString(data, r->dn, strlen(r->dn))) goto err; + if (!asn1_push_tag(data, ASN1_SEQUENCE(0))) goto err; for (i=0; inum_attributes; i++) { struct ldb_message_element *attrib = &r->attributes[i]; - asn1_push_tag(data, ASN1_SEQUENCE(0)); - asn1_write_OctetString(data, attrib->name, - strlen(attrib->name)); - asn1_push_tag(data, ASN1_SET); + if (!asn1_push_tag(data, ASN1_SEQUENCE(0))) goto err; + if (!asn1_write_OctetString(data, attrib->name, + strlen(attrib->name))) goto err; + if (!asn1_push_tag(data, ASN1_SET)) goto err; for (j=0; jattributes[i].num_values; j++) { - asn1_write_OctetString(data, + if (!asn1_write_OctetString(data, attrib->values[j].data, - attrib->values[j].length); + attrib->values[j].length)) goto err; } - asn1_pop_tag(data); - asn1_pop_tag(data); + if (!asn1_pop_tag(data)) goto err; + if (!asn1_pop_tag(data)) goto err; } - asn1_pop_tag(data); - asn1_pop_tag(data); + if (!asn1_pop_tag(data)) goto err; + if (!asn1_pop_tag(data)) goto err; break; } case LDAP_TAG_AddResponse: { struct ldap_Result *r = &msg->r.AddResponse; - asn1_push_tag(data, ASN1_APPLICATION(msg->type)); - ldap_encode_response(data, r); - asn1_pop_tag(data); + if (!asn1_push_tag(data, ASN1_APPLICATION(msg->type))) goto err; + if (!ldap_encode_response(data, r)) goto err; + if (!asn1_pop_tag(data)) goto err; break; } case LDAP_TAG_DelRequest: { struct ldap_DelRequest *r = &msg->r.DelRequest; - asn1_push_tag(data, ASN1_APPLICATION_SIMPLE(msg->type)); - asn1_write(data, r->dn, strlen(r->dn)); - asn1_pop_tag(data); + if (!asn1_push_tag(data, ASN1_APPLICATION_SIMPLE(msg->type))) goto err; + if (!asn1_write(data, r->dn, strlen(r->dn))) goto err; + if (!asn1_pop_tag(data)) goto err; break; } case LDAP_TAG_DelResponse: { struct ldap_Result *r = &msg->r.DelResponse; - asn1_push_tag(data, ASN1_APPLICATION(msg->type)); - ldap_encode_response(data, r); - asn1_pop_tag(data); + if (!asn1_push_tag(data, ASN1_APPLICATION(msg->type))) goto err; + if (!ldap_encode_response(data, r)) goto err; + if (!asn1_pop_tag(data)) goto err; break; } case LDAP_TAG_ModifyDNRequest: { struct ldap_ModifyDNRequest *r = &msg->r.ModifyDNRequest; - asn1_push_tag(data, ASN1_APPLICATION(msg->type)); - asn1_write_OctetString(data, r->dn, strlen(r->dn)); - asn1_write_OctetString(data, r->newrdn, strlen(r->newrdn)); - asn1_write_BOOLEAN(data, r->deleteolddn); + if (!asn1_push_tag(data, ASN1_APPLICATION(msg->type))) goto err; + if (!asn1_write_OctetString(data, r->dn, strlen(r->dn))) goto err; + if (!asn1_write_OctetString(data, r->newrdn, strlen(r->newrdn))) goto err; + if (!asn1_write_BOOLEAN(data, r->deleteolddn)) goto err; if (r->newsuperior) { - asn1_push_tag(data, ASN1_CONTEXT_SIMPLE(0)); - asn1_write(data, r->newsuperior, - strlen(r->newsuperior)); - asn1_pop_tag(data); + if (!asn1_push_tag(data, ASN1_CONTEXT_SIMPLE(0))) goto err; + if (!asn1_write(data, r->newsuperior, + strlen(r->newsuperior))) goto err; + if (!asn1_pop_tag(data)) goto err; } - asn1_pop_tag(data); + if (!asn1_pop_tag(data)) goto err; break; } case LDAP_TAG_ModifyDNResponse: { struct ldap_Result *r = &msg->r.ModifyDNResponse; - asn1_push_tag(data, ASN1_APPLICATION(msg->type)); - ldap_encode_response(data, r); - asn1_pop_tag(data); + if (!asn1_push_tag(data, ASN1_APPLICATION(msg->type))) goto err; + if (!ldap_encode_response(data, r)) goto err; + if (!asn1_pop_tag(data)) goto err; break; } case LDAP_TAG_CompareRequest: { struct ldap_CompareRequest *r = &msg->r.CompareRequest; - asn1_push_tag(data, ASN1_APPLICATION(msg->type)); - asn1_write_OctetString(data, r->dn, strlen(r->dn)); - asn1_push_tag(data, ASN1_SEQUENCE(0)); - asn1_write_OctetString(data, r->attribute, - strlen(r->attribute)); - asn1_write_OctetString(data, r->value.data, - r->value.length); - asn1_pop_tag(data); - asn1_pop_tag(data); + if (!asn1_push_tag(data, ASN1_APPLICATION(msg->type))) goto err; + if (!asn1_write_OctetString(data, r->dn, strlen(r->dn))) goto err; + if (!asn1_push_tag(data, ASN1_SEQUENCE(0))) goto err; + if (!asn1_write_OctetString(data, r->attribute, + strlen(r->attribute))) goto err; + if (!asn1_write_OctetString(data, r->value.data, + r->value.length)) goto err; + if (!asn1_pop_tag(data)) goto err; + if (!asn1_pop_tag(data)) goto err; break; } case LDAP_TAG_CompareResponse: { struct ldap_Result *r = &msg->r.ModifyDNResponse; - asn1_push_tag(data, ASN1_APPLICATION(msg->type)); - ldap_encode_response(data, r); - asn1_pop_tag(data); + if (!asn1_push_tag(data, ASN1_APPLICATION(msg->type))) goto err; + if (!ldap_encode_response(data, r)) goto err; + if (!asn1_pop_tag(data)) goto err; break; } case LDAP_TAG_AbandonRequest: { struct ldap_AbandonRequest *r = &msg->r.AbandonRequest; - asn1_push_tag(data, ASN1_APPLICATION_SIMPLE(msg->type)); - asn1_write_implicit_Integer(data, r->messageid); - asn1_pop_tag(data); + if (!asn1_push_tag(data, ASN1_APPLICATION_SIMPLE(msg->type))) goto err; + if (!asn1_write_implicit_Integer(data, r->messageid)) goto err; + if (!asn1_pop_tag(data)) goto err; break; } case LDAP_TAG_SearchResultReference: { struct ldap_SearchResRef *r = &msg->r.SearchResultReference; - asn1_push_tag(data, ASN1_APPLICATION(msg->type)); - asn1_write_OctetString(data, r->referral, strlen(r->referral)); - asn1_pop_tag(data); + if (!asn1_push_tag(data, ASN1_APPLICATION(msg->type))) goto err; + if (!asn1_write_OctetString(data, r->referral, strlen(r->referral))) goto err; + if (!asn1_pop_tag(data)) goto err; break; } case LDAP_TAG_ExtendedRequest: { struct ldap_ExtendedRequest *r = &msg->r.ExtendedRequest; - asn1_push_tag(data, ASN1_APPLICATION(msg->type)); - asn1_push_tag(data, ASN1_CONTEXT_SIMPLE(0)); - asn1_write(data, r->oid, strlen(r->oid)); - asn1_pop_tag(data); + if (!asn1_push_tag(data, ASN1_APPLICATION(msg->type))) goto err; + if (!asn1_push_tag(data, ASN1_CONTEXT_SIMPLE(0))) goto err; + if (!asn1_write(data, r->oid, strlen(r->oid))) goto err; + if (!asn1_pop_tag(data)) goto err; if (r->value) { - asn1_push_tag(data, ASN1_CONTEXT_SIMPLE(1)); - asn1_write(data, r->value->data, r->value->length); - asn1_pop_tag(data); + if (!asn1_push_tag(data, ASN1_CONTEXT_SIMPLE(1))) goto err; + if (!asn1_write(data, r->value->data, r->value->length)) goto err; + if (!asn1_pop_tag(data)) goto err; } - asn1_pop_tag(data); + if (!asn1_pop_tag(data)) goto err; break; } case LDAP_TAG_ExtendedResponse: { struct ldap_ExtendedResponse *r = &msg->r.ExtendedResponse; - asn1_push_tag(data, ASN1_APPLICATION(msg->type)); - ldap_encode_response(data, &r->response); + if (!asn1_push_tag(data, ASN1_APPLICATION(msg->type))) goto err; + if (!ldap_encode_response(data, &r->response)) goto err; if (r->oid) { - asn1_push_tag(data, ASN1_CONTEXT_SIMPLE(10)); - asn1_write(data, r->oid, strlen(r->oid)); - asn1_pop_tag(data); + if (!asn1_push_tag(data, ASN1_CONTEXT_SIMPLE(10))) goto err; + if (!asn1_write(data, r->oid, strlen(r->oid))) goto err; + if (!asn1_pop_tag(data)) goto err; } if (r->value) { - asn1_push_tag(data, ASN1_CONTEXT_SIMPLE(11)); - asn1_write(data, r->value->data, r->value->length); - asn1_pop_tag(data); + if (!asn1_push_tag(data, ASN1_CONTEXT_SIMPLE(11))) goto err; + if (!asn1_write(data, r->value->data, r->value->length)) goto err; + if (!asn1_pop_tag(data)) goto err; } - asn1_pop_tag(data); + if (!asn1_pop_tag(data)) goto err; break; } default: - return false; + goto err; } if (msg->controls != NULL) { - asn1_push_tag(data, ASN1_CONTEXT(0)); + if (!asn1_push_tag(data, ASN1_CONTEXT(0))) goto err; for (i = 0; msg->controls[i] != NULL; i++) { if (!ldap_encode_control(mem_ctx, data, @@ -681,23 +682,24 @@ _PUBLIC_ bool ldap_encode(struct ldap_message *msg, msg->controls[i])) { DEBUG(0,("Unable to encode control %s\n", msg->controls[i]->oid)); - return false; + goto err; } } - asn1_pop_tag(data); + if (!asn1_pop_tag(data)) goto err; } - asn1_pop_tag(data); - - if (data->has_error) { - asn1_free(data); - return false; - } + if (!asn1_pop_tag(data)) goto err; *result = data_blob_talloc(mem_ctx, data->data, data->length); asn1_free(data); + return true; + + err: + + asn1_free(data); + return false; } static const char *blob2string_talloc(TALLOC_CTX *mem_ctx, @@ -721,20 +723,21 @@ bool asn1_read_OctetString_talloc(TALLOC_CTX *mem_ctx, return true; } -static void ldap_decode_response(TALLOC_CTX *mem_ctx, +static bool ldap_decode_response(TALLOC_CTX *mem_ctx, struct asn1_data *data, struct ldap_Result *result) { - asn1_read_enumerated(data, &result->resultcode); - asn1_read_OctetString_talloc(mem_ctx, data, &result->dn); - asn1_read_OctetString_talloc(mem_ctx, data, &result->errormessage); + if (!asn1_read_enumerated(data, &result->resultcode)) return false; + if (!asn1_read_OctetString_talloc(mem_ctx, data, &result->dn)) return false; + if (!asn1_read_OctetString_talloc(mem_ctx, data, &result->errormessage)) return false; if (asn1_peek_tag(data, ASN1_CONTEXT(3))) { - asn1_start_tag(data, ASN1_CONTEXT(3)); - asn1_read_OctetString_talloc(mem_ctx, data, &result->referral); - asn1_end_tag(data); + if (!asn1_start_tag(data, ASN1_CONTEXT(3))) return false; + if (!asn1_read_OctetString_talloc(mem_ctx, data, &result->referral)) return false; + if (!asn1_end_tag(data)) return false; } else { result->referral = NULL; } + return true; } static struct ldb_val **ldap_decode_substring(TALLOC_CTX *mem_ctx, struct ldb_val **chunks, int chunk_num, char *value) @@ -835,10 +838,10 @@ static struct ldb_parse_tree *ldap_decode_filter_tree(TALLOC_CTX *mem_ctx, const char *attrib; DATA_BLOB value; - asn1_start_tag(data, ASN1_CONTEXT(filter_tag)); - asn1_read_OctetString_talloc(mem_ctx, data, &attrib); - asn1_read_OctetString(data, mem_ctx, &value); - asn1_end_tag(data); + if (!asn1_start_tag(data, ASN1_CONTEXT(filter_tag))) goto failed; + if (!asn1_read_OctetString_talloc(mem_ctx, data, &attrib)) goto failed; + if (!asn1_read_OctetString(data, mem_ctx, &value)) goto failed; + if (!asn1_end_tag(data)) goto failed; if ((data->has_error) || (attrib == NULL) || (value.data == NULL)) { goto failed; } @@ -874,13 +877,13 @@ static struct ldb_parse_tree *ldap_decode_filter_tree(TALLOC_CTX *mem_ctx, } while (asn1_tag_remaining(data)) { - asn1_peek_uint8(data, &subs_tag); + if (!asn1_peek_uint8(data, &subs_tag)) goto failed; subs_tag &= 0x1f; /* strip off the asn1 stuff */ if (subs_tag > 2) goto failed; - asn1_start_tag(data, ASN1_CONTEXT_SIMPLE(subs_tag)); - asn1_read_LDAPString(data, mem_ctx, &value); - asn1_end_tag(data); + if (!asn1_start_tag(data, ASN1_CONTEXT_SIMPLE(subs_tag))) goto failed; + if (!asn1_read_LDAPString(data, mem_ctx, &value)) goto failed; + if (!asn1_end_tag(data)) goto failed; switch (subs_tag) { case 0: @@ -947,10 +950,10 @@ static struct ldb_parse_tree *ldap_decode_filter_tree(TALLOC_CTX *mem_ctx, const char *attrib; DATA_BLOB value; - asn1_start_tag(data, ASN1_CONTEXT(filter_tag)); - asn1_read_OctetString_talloc(mem_ctx, data, &attrib); - asn1_read_OctetString(data, mem_ctx, &value); - asn1_end_tag(data); + if (!asn1_start_tag(data, ASN1_CONTEXT(filter_tag))) goto failed; + if (!asn1_read_OctetString_talloc(mem_ctx, data, &attrib)) goto failed; + if (!asn1_read_OctetString(data, mem_ctx, &value)) goto failed; + if (!asn1_end_tag(data)) goto failed; if ((data->has_error) || (attrib == NULL) || (value.data == NULL)) { goto failed; } @@ -966,10 +969,10 @@ static struct ldb_parse_tree *ldap_decode_filter_tree(TALLOC_CTX *mem_ctx, const char *attrib; DATA_BLOB value; - asn1_start_tag(data, ASN1_CONTEXT(filter_tag)); - asn1_read_OctetString_talloc(mem_ctx, data, &attrib); - asn1_read_OctetString(data, mem_ctx, &value); - asn1_end_tag(data); + if (!asn1_start_tag(data, ASN1_CONTEXT(filter_tag))) goto failed; + if (!asn1_read_OctetString_talloc(mem_ctx, data, &attrib)) goto failed; + if (!asn1_read_OctetString(data, mem_ctx, &value)) goto failed; + if (!asn1_end_tag(data)) goto failed; if ((data->has_error) || (attrib == NULL) || (value.data == NULL)) { goto failed; } @@ -1004,10 +1007,10 @@ static struct ldb_parse_tree *ldap_decode_filter_tree(TALLOC_CTX *mem_ctx, const char *attrib; DATA_BLOB value; - asn1_start_tag(data, ASN1_CONTEXT(filter_tag)); - asn1_read_OctetString_talloc(mem_ctx, data, &attrib); - asn1_read_OctetString(data, mem_ctx, &value); - asn1_end_tag(data); + if (!asn1_start_tag(data, ASN1_CONTEXT(filter_tag))) goto failed; + if (!asn1_read_OctetString_talloc(mem_ctx, data, &attrib)) goto failed; + if (!asn1_read_OctetString(data, mem_ctx, &value)) goto failed; + if (!asn1_end_tag(data)) goto failed; if ((data->has_error) || (attrib == NULL) || (value.data == NULL)) { goto failed; } @@ -1030,18 +1033,18 @@ static struct ldb_parse_tree *ldap_decode_filter_tree(TALLOC_CTX *mem_ctx, we need to check we properly implement --SSS */ /* either oid or type must be defined */ if (asn1_peek_tag(data, ASN1_CONTEXT_SIMPLE(1))) { /* optional */ - asn1_start_tag(data, ASN1_CONTEXT_SIMPLE(1)); - asn1_read_LDAPString(data, ret, &oid); - asn1_end_tag(data); + if (!asn1_start_tag(data, ASN1_CONTEXT_SIMPLE(1))) goto failed; + if (!asn1_read_LDAPString(data, ret, &oid)) goto failed; + if (!asn1_end_tag(data)) goto failed; } if (asn1_peek_tag(data, ASN1_CONTEXT_SIMPLE(2))) { /* optional */ - asn1_start_tag(data, ASN1_CONTEXT_SIMPLE(2)); - asn1_read_LDAPString(data, ret, &attr); - asn1_end_tag(data); + if (!asn1_start_tag(data, ASN1_CONTEXT_SIMPLE(2))) goto failed; + if (!asn1_read_LDAPString(data, ret, &attr)) goto failed; + if (!asn1_end_tag(data)) goto failed; } - asn1_start_tag(data, ASN1_CONTEXT_SIMPLE(3)); - asn1_read_LDAPString(data, ret, &value); - asn1_end_tag(data); + if (!asn1_start_tag(data, ASN1_CONTEXT_SIMPLE(3))) goto failed; + if (!asn1_read_LDAPString(data, ret, &value)) goto failed; + if (!asn1_end_tag(data)) goto failed; /* dnAttributes is marked as BOOLEAN DEFAULT FALSE it is not marked as OPTIONAL but openldap tools do not set this unless it is to be set as TRUE @@ -1049,9 +1052,9 @@ static struct ldb_parse_tree *ldap_decode_filter_tree(TALLOC_CTX *mem_ctx, seems that AD always requires the dnAttributes boolean value to be set */ if (asn1_peek_tag(data, ASN1_CONTEXT_SIMPLE(4))) { - asn1_start_tag(data, ASN1_CONTEXT_SIMPLE(4)); - asn1_read_uint8(data, &dnAttributes); - asn1_end_tag(data); + if (!asn1_start_tag(data, ASN1_CONTEXT_SIMPLE(4))) goto failed; + if (!asn1_read_uint8(data, &dnAttributes)) goto failed; + if (!asn1_end_tag(data)) goto failed; } else { dnAttributes = 0; } @@ -1099,45 +1102,45 @@ failed: } /* Decode a single LDAP attribute, possibly containing multiple values */ -static void ldap_decode_attrib(TALLOC_CTX *mem_ctx, struct asn1_data *data, +static bool ldap_decode_attrib(TALLOC_CTX *mem_ctx, struct asn1_data *data, struct ldb_message_element *attrib) { - asn1_start_tag(data, ASN1_SEQUENCE(0)); - asn1_read_OctetString_talloc(mem_ctx, data, &attrib->name); - asn1_start_tag(data, ASN1_SET); + if (!asn1_start_tag(data, ASN1_SEQUENCE(0))) return false; + if (!asn1_read_OctetString_talloc(mem_ctx, data, &attrib->name)) return false; + if (!asn1_start_tag(data, ASN1_SET)) return false; while (asn1_peek_tag(data, ASN1_OCTET_STRING)) { DATA_BLOB blob; - asn1_read_OctetString(data, mem_ctx, &blob); + if (!asn1_read_OctetString(data, mem_ctx, &blob)) return false; add_value_to_attrib(mem_ctx, &blob, attrib); } - asn1_end_tag(data); - asn1_end_tag(data); - + if (!asn1_end_tag(data)) return false; + return asn1_end_tag(data); } /* Decode a set of LDAP attributes, as found in the dereference control */ -void ldap_decode_attribs_bare(TALLOC_CTX *mem_ctx, struct asn1_data *data, +bool ldap_decode_attribs_bare(TALLOC_CTX *mem_ctx, struct asn1_data *data, struct ldb_message_element **attributes, int *num_attributes) { while (asn1_peek_tag(data, ASN1_SEQUENCE(0))) { struct ldb_message_element attrib; ZERO_STRUCT(attrib); - ldap_decode_attrib(mem_ctx, data, &attrib); + if (!ldap_decode_attrib(mem_ctx, data, &attrib)) return false; add_attrib_to_array_talloc(mem_ctx, &attrib, attributes, num_attributes); } + return true; } /* Decode a set of LDAP attributes, as found in a search entry */ -static void ldap_decode_attribs(TALLOC_CTX *mem_ctx, struct asn1_data *data, +static bool ldap_decode_attribs(TALLOC_CTX *mem_ctx, struct asn1_data *data, struct ldb_message_element **attributes, int *num_attributes) { - asn1_start_tag(data, ASN1_SEQUENCE(0)); - ldap_decode_attribs_bare(mem_ctx, data, - attributes, num_attributes); - asn1_end_tag(data); + if (!asn1_start_tag(data, ASN1_SEQUENCE(0))) return false; + if (!ldap_decode_attribs_bare(mem_ctx, data, + attributes, num_attributes)) return false; + return asn1_end_tag(data); } /* This routine returns LDAP status codes */ @@ -1148,46 +1151,45 @@ _PUBLIC_ NTSTATUS ldap_decode(struct asn1_data *data, { uint8_t tag; - asn1_start_tag(data, ASN1_SEQUENCE(0)); - asn1_read_Integer(data, &msg->messageid); + if (!asn1_start_tag(data, ASN1_SEQUENCE(0))) goto prot_err; + if (!asn1_read_Integer(data, &msg->messageid)) goto prot_err; - if (!asn1_peek_uint8(data, &tag)) - return NT_STATUS_LDAP(LDAP_PROTOCOL_ERROR); + if (!asn1_peek_uint8(data, &tag)) goto prot_err; switch(tag) { case ASN1_APPLICATION(LDAP_TAG_BindRequest): { struct ldap_BindRequest *r = &msg->r.BindRequest; msg->type = LDAP_TAG_BindRequest; - asn1_start_tag(data, tag); - asn1_read_Integer(data, &r->version); - asn1_read_OctetString_talloc(msg, data, &r->dn); + if (!asn1_start_tag(data, tag)) goto prot_err; + if (!asn1_read_Integer(data, &r->version)) goto prot_err; + if (!asn1_read_OctetString_talloc(msg, data, &r->dn)) goto prot_err; if (asn1_peek_tag(data, ASN1_CONTEXT_SIMPLE(0))) { int pwlen; r->creds.password = ""; r->mechanism = LDAP_AUTH_MECH_SIMPLE; - asn1_start_tag(data, ASN1_CONTEXT_SIMPLE(0)); + if (!asn1_start_tag(data, ASN1_CONTEXT_SIMPLE(0))) goto prot_err; pwlen = asn1_tag_remaining(data); if (pwlen == -1) { - return NT_STATUS_LDAP(LDAP_PROTOCOL_ERROR); + goto prot_err; } if (pwlen != 0) { char *pw = talloc_array(msg, char, pwlen+1); if (!pw) { return NT_STATUS_LDAP(LDAP_OPERATIONS_ERROR); } - asn1_read(data, pw, pwlen); + if (!asn1_read(data, pw, pwlen)) goto prot_err; pw[pwlen] = '\0'; r->creds.password = pw; } - asn1_end_tag(data); + if (!asn1_end_tag(data)) goto prot_err; } else if (asn1_peek_tag(data, ASN1_CONTEXT(3))){ - asn1_start_tag(data, ASN1_CONTEXT(3)); + if (!asn1_start_tag(data, ASN1_CONTEXT(3))) goto prot_err; r->mechanism = LDAP_AUTH_MECH_SASL; - asn1_read_OctetString_talloc(msg, data, &r->creds.SASL.mechanism); + if (!asn1_read_OctetString_talloc(msg, data, &r->creds.SASL.mechanism)) goto prot_err; if (asn1_peek_tag(data, ASN1_OCTET_STRING)) { /* optional */ DATA_BLOB tmp_blob = data_blob(NULL, 0); - asn1_read_OctetString(data, msg, &tmp_blob); + if (!asn1_read_OctetString(data, msg, &tmp_blob)) goto prot_err; r->creds.SASL.secblob = talloc(msg, DATA_BLOB); if (!r->creds.SASL.secblob) { return NT_STATUS_LDAP(LDAP_OPERATIONS_ERROR); @@ -1198,23 +1200,23 @@ _PUBLIC_ NTSTATUS ldap_decode(struct asn1_data *data, } else { r->creds.SASL.secblob = NULL; } - asn1_end_tag(data); + if (!asn1_end_tag(data)) goto prot_err; } else { /* Neither Simple nor SASL bind */ - return NT_STATUS_LDAP(LDAP_PROTOCOL_ERROR); + goto prot_err; } - asn1_end_tag(data); + if (!asn1_end_tag(data)) goto prot_err; break; } case ASN1_APPLICATION(LDAP_TAG_BindResponse): { struct ldap_BindResponse *r = &msg->r.BindResponse; msg->type = LDAP_TAG_BindResponse; - asn1_start_tag(data, tag); - ldap_decode_response(msg, data, &r->response); + if (!asn1_start_tag(data, tag)) goto prot_err; + if (!ldap_decode_response(msg, data, &r->response)) goto prot_err; if (asn1_peek_tag(data, ASN1_CONTEXT_SIMPLE(7))) { DATA_BLOB tmp_blob = data_blob(NULL, 0); - asn1_read_ContextSimple(data, 7, &tmp_blob); + if (!asn1_read_ContextSimple(data, 7, &tmp_blob)) goto prot_err; r->SASL.secblob = talloc(msg, DATA_BLOB); if (!r->SASL.secblob) { return NT_STATUS_LDAP(LDAP_OPERATIONS_ERROR); @@ -1225,14 +1227,14 @@ _PUBLIC_ NTSTATUS ldap_decode(struct asn1_data *data, } else { r->SASL.secblob = NULL; } - asn1_end_tag(data); + if (!asn1_end_tag(data)) goto prot_err; break; } case ASN1_APPLICATION_SIMPLE(LDAP_TAG_UnbindRequest): { msg->type = LDAP_TAG_UnbindRequest; - asn1_start_tag(data, tag); - asn1_end_tag(data); + if (!asn1_start_tag(data, tag)) goto prot_err; + if (!asn1_end_tag(data)) goto prot_err; break; } @@ -1241,41 +1243,41 @@ _PUBLIC_ NTSTATUS ldap_decode(struct asn1_data *data, int sizelimit, timelimit; const char **attrs = NULL; msg->type = LDAP_TAG_SearchRequest; - asn1_start_tag(data, tag); - asn1_read_OctetString_talloc(msg, data, &r->basedn); - asn1_read_enumerated(data, (int *)(void *)&(r->scope)); - asn1_read_enumerated(data, (int *)(void *)&(r->deref)); - asn1_read_Integer(data, &sizelimit); + if (!asn1_start_tag(data, tag)) goto prot_err; + if (!asn1_read_OctetString_talloc(msg, data, &r->basedn)) goto prot_err; + if (!asn1_read_enumerated(data, (int *)(void *)&(r->scope))) goto prot_err; + if (!asn1_read_enumerated(data, (int *)(void *)&(r->deref))) goto prot_err; + if (!asn1_read_Integer(data, &sizelimit)) goto prot_err; r->sizelimit = sizelimit; - asn1_read_Integer(data, &timelimit); + if (!asn1_read_Integer(data, &timelimit)) goto prot_err; r->timelimit = timelimit; - asn1_read_BOOLEAN(data, &r->attributesonly); + if (!asn1_read_BOOLEAN(data, &r->attributesonly)) goto prot_err; r->tree = ldap_decode_filter_tree(msg, data); if (r->tree == NULL) { - return NT_STATUS_LDAP(LDAP_PROTOCOL_ERROR); + goto prot_err; } - asn1_start_tag(data, ASN1_SEQUENCE(0)); + if (!asn1_start_tag(data, ASN1_SEQUENCE(0))) goto prot_err; r->num_attributes = 0; r->attributes = NULL; - while (asn1_tag_remaining(data) > 0) { + while (asn1_tag_remaining(data) > 0) { const char *attr; if (!asn1_read_OctetString_talloc(msg, data, &attr)) - return NT_STATUS_LDAP(LDAP_PROTOCOL_ERROR); + goto prot_err; if (!add_string_to_array(msg, attr, &attrs, &r->num_attributes)) - return NT_STATUS_LDAP(LDAP_PROTOCOL_ERROR); + goto prot_err; } r->attributes = attrs; - asn1_end_tag(data); - asn1_end_tag(data); + if (!asn1_end_tag(data)) goto prot_err; + if (!asn1_end_tag(data)) goto prot_err; break; } @@ -1284,38 +1286,38 @@ _PUBLIC_ NTSTATUS ldap_decode(struct asn1_data *data, msg->type = LDAP_TAG_SearchResultEntry; r->attributes = NULL; r->num_attributes = 0; - asn1_start_tag(data, tag); - asn1_read_OctetString_talloc(msg, data, &r->dn); - ldap_decode_attribs(msg, data, &r->attributes, - &r->num_attributes); - asn1_end_tag(data); + if (!asn1_start_tag(data, tag)) goto prot_err; + if (!asn1_read_OctetString_talloc(msg, data, &r->dn)) goto prot_err; + if (!ldap_decode_attribs(msg, data, &r->attributes, + &r->num_attributes)) goto prot_err; + if (!asn1_end_tag(data)) goto prot_err; break; } case ASN1_APPLICATION(LDAP_TAG_SearchResultDone): { struct ldap_Result *r = &msg->r.SearchResultDone; msg->type = LDAP_TAG_SearchResultDone; - asn1_start_tag(data, tag); - ldap_decode_response(msg, data, r); - asn1_end_tag(data); + if (!asn1_start_tag(data, tag)) goto prot_err; + if (!ldap_decode_response(msg, data, r)) goto prot_err; + if (!asn1_end_tag(data)) goto prot_err; break; } case ASN1_APPLICATION(LDAP_TAG_SearchResultReference): { struct ldap_SearchResRef *r = &msg->r.SearchResultReference; msg->type = LDAP_TAG_SearchResultReference; - asn1_start_tag(data, tag); - asn1_read_OctetString_talloc(msg, data, &r->referral); - asn1_end_tag(data); + if (!asn1_start_tag(data, tag)) goto prot_err; + if (!asn1_read_OctetString_talloc(msg, data, &r->referral)) goto prot_err; + if (!asn1_end_tag(data)) goto prot_err; break; } case ASN1_APPLICATION(LDAP_TAG_ModifyRequest): { struct ldap_ModifyRequest *r = &msg->r.ModifyRequest; msg->type = LDAP_TAG_ModifyRequest; - asn1_start_tag(data, ASN1_APPLICATION(LDAP_TAG_ModifyRequest)); - asn1_read_OctetString_talloc(msg, data, &r->dn); - asn1_start_tag(data, ASN1_SEQUENCE(0)); + if (!asn1_start_tag(data, ASN1_APPLICATION(LDAP_TAG_ModifyRequest))) goto prot_err; + if (!asn1_read_OctetString_talloc(msg, data, &r->dn)) goto prot_err; + if (!asn1_start_tag(data, ASN1_SEQUENCE(0))) goto prot_err; r->num_mods = 0; r->mods = NULL; @@ -1324,52 +1326,52 @@ _PUBLIC_ NTSTATUS ldap_decode(struct asn1_data *data, struct ldap_mod mod; int v; ZERO_STRUCT(mod); - asn1_start_tag(data, ASN1_SEQUENCE(0)); - asn1_read_enumerated(data, &v); + if (!asn1_start_tag(data, ASN1_SEQUENCE(0))) goto prot_err; + if (!asn1_read_enumerated(data, &v)) goto prot_err; mod.type = v; - ldap_decode_attrib(msg, data, &mod.attrib); - asn1_end_tag(data); + if (!ldap_decode_attrib(msg, data, &mod.attrib)) goto prot_err; + if (!asn1_end_tag(data)) goto prot_err; if (!add_mod_to_array_talloc(msg, &mod, &r->mods, &r->num_mods)) { return NT_STATUS_LDAP(LDAP_OPERATIONS_ERROR); } } - asn1_end_tag(data); - asn1_end_tag(data); + if (!asn1_end_tag(data)) goto prot_err; + if (!asn1_end_tag(data)) goto prot_err; break; } case ASN1_APPLICATION(LDAP_TAG_ModifyResponse): { struct ldap_Result *r = &msg->r.ModifyResponse; msg->type = LDAP_TAG_ModifyResponse; - asn1_start_tag(data, tag); - ldap_decode_response(msg, data, r); - asn1_end_tag(data); + if (!asn1_start_tag(data, tag)) goto prot_err; + if (!ldap_decode_response(msg, data, r)) goto prot_err; + if (!asn1_end_tag(data)) goto prot_err; break; } case ASN1_APPLICATION(LDAP_TAG_AddRequest): { struct ldap_AddRequest *r = &msg->r.AddRequest; msg->type = LDAP_TAG_AddRequest; - asn1_start_tag(data, tag); - asn1_read_OctetString_talloc(msg, data, &r->dn); + if (!asn1_start_tag(data, tag)) goto prot_err; + if (!asn1_read_OctetString_talloc(msg, data, &r->dn)) goto prot_err; r->attributes = NULL; r->num_attributes = 0; - ldap_decode_attribs(msg, data, &r->attributes, - &r->num_attributes); + if (!ldap_decode_attribs(msg, data, &r->attributes, + &r->num_attributes)) goto prot_err; - asn1_end_tag(data); + if (!asn1_end_tag(data)) goto prot_err; break; } case ASN1_APPLICATION(LDAP_TAG_AddResponse): { struct ldap_Result *r = &msg->r.AddResponse; msg->type = LDAP_TAG_AddResponse; - asn1_start_tag(data, tag); - ldap_decode_response(msg, data, r); - asn1_end_tag(data); + if (!asn1_start_tag(data, tag)) goto prot_err; + if (!ldap_decode_response(msg, data, r)) goto prot_err; + if (!asn1_end_tag(data)) goto prot_err; break; } @@ -1378,102 +1380,102 @@ _PUBLIC_ NTSTATUS ldap_decode(struct asn1_data *data, int len; char *dn; msg->type = LDAP_TAG_DelRequest; - asn1_start_tag(data, - ASN1_APPLICATION_SIMPLE(LDAP_TAG_DelRequest)); + if (!asn1_start_tag(data, + ASN1_APPLICATION_SIMPLE(LDAP_TAG_DelRequest))) goto prot_err; len = asn1_tag_remaining(data); if (len == -1) { - return NT_STATUS_LDAP(LDAP_PROTOCOL_ERROR); + goto prot_err; } dn = talloc_array(msg, char, len+1); if (dn == NULL) break; - asn1_read(data, dn, len); + if (!asn1_read(data, dn, len)) goto prot_err; dn[len] = '\0'; r->dn = dn; - asn1_end_tag(data); + if (!asn1_end_tag(data)) goto prot_err; break; } case ASN1_APPLICATION(LDAP_TAG_DelResponse): { struct ldap_Result *r = &msg->r.DelResponse; msg->type = LDAP_TAG_DelResponse; - asn1_start_tag(data, tag); - ldap_decode_response(msg, data, r); - asn1_end_tag(data); + if (!asn1_start_tag(data, tag)) goto prot_err; + if (!ldap_decode_response(msg, data, r)) goto prot_err; + if (!asn1_end_tag(data)) goto prot_err; break; } case ASN1_APPLICATION(LDAP_TAG_ModifyDNRequest): { struct ldap_ModifyDNRequest *r = &msg->r.ModifyDNRequest; msg->type = LDAP_TAG_ModifyDNRequest; - asn1_start_tag(data, - ASN1_APPLICATION(LDAP_TAG_ModifyDNRequest)); - asn1_read_OctetString_talloc(msg, data, &r->dn); - asn1_read_OctetString_talloc(msg, data, &r->newrdn); - asn1_read_BOOLEAN(data, &r->deleteolddn); + if (!asn1_start_tag(data, + ASN1_APPLICATION(LDAP_TAG_ModifyDNRequest))) goto prot_err; + if (!asn1_read_OctetString_talloc(msg, data, &r->dn)) goto prot_err; + if (!asn1_read_OctetString_talloc(msg, data, &r->newrdn)) goto prot_err; + if (!asn1_read_BOOLEAN(data, &r->deleteolddn)) goto prot_err; r->newsuperior = NULL; if (asn1_tag_remaining(data) > 0) { int len; char *newsup; - asn1_start_tag(data, ASN1_CONTEXT_SIMPLE(0)); + if (!asn1_start_tag(data, ASN1_CONTEXT_SIMPLE(0))) goto prot_err; len = asn1_tag_remaining(data); if (len == -1) { - return NT_STATUS_LDAP(LDAP_PROTOCOL_ERROR); + goto prot_err; } newsup = talloc_array(msg, char, len+1); if (newsup == NULL) { return NT_STATUS_LDAP(LDAP_OPERATIONS_ERROR); } - asn1_read(data, newsup, len); + if (!asn1_read(data, newsup, len)) goto prot_err; newsup[len] = '\0'; r->newsuperior = newsup; - asn1_end_tag(data); + if (!asn1_end_tag(data)) goto prot_err; } - asn1_end_tag(data); + if (!asn1_end_tag(data)) goto prot_err; break; } case ASN1_APPLICATION(LDAP_TAG_ModifyDNResponse): { struct ldap_Result *r = &msg->r.ModifyDNResponse; msg->type = LDAP_TAG_ModifyDNResponse; - asn1_start_tag(data, tag); - ldap_decode_response(msg, data, r); - asn1_end_tag(data); + if (!asn1_start_tag(data, tag)) goto prot_err; + if (!ldap_decode_response(msg, data, r)) goto prot_err; + if (!asn1_end_tag(data)) goto prot_err; break; } case ASN1_APPLICATION(LDAP_TAG_CompareRequest): { struct ldap_CompareRequest *r = &msg->r.CompareRequest; msg->type = LDAP_TAG_CompareRequest; - asn1_start_tag(data, - ASN1_APPLICATION(LDAP_TAG_CompareRequest)); - asn1_read_OctetString_talloc(msg, data, &r->dn); - asn1_start_tag(data, ASN1_SEQUENCE(0)); - asn1_read_OctetString_talloc(msg, data, &r->attribute); - asn1_read_OctetString(data, msg, &r->value); + if (!asn1_start_tag(data, + ASN1_APPLICATION(LDAP_TAG_CompareRequest))) goto prot_err; + if (!asn1_read_OctetString_talloc(msg, data, &r->dn)) goto prot_err; + if (!asn1_start_tag(data, ASN1_SEQUENCE(0))) goto prot_err; + if (!asn1_read_OctetString_talloc(msg, data, &r->attribute)) goto prot_err; + if (!asn1_read_OctetString(data, msg, &r->value)) goto prot_err; if (r->value.data) { talloc_steal(msg, r->value.data); } - asn1_end_tag(data); - asn1_end_tag(data); + if (!asn1_end_tag(data)) goto prot_err; + if (!asn1_end_tag(data)) goto prot_err; break; } case ASN1_APPLICATION(LDAP_TAG_CompareResponse): { struct ldap_Result *r = &msg->r.CompareResponse; msg->type = LDAP_TAG_CompareResponse; - asn1_start_tag(data, tag); - ldap_decode_response(msg, data, r); - asn1_end_tag(data); + if (!asn1_start_tag(data, tag)) goto prot_err; + if (!ldap_decode_response(msg, data, r)) goto prot_err; + if (!asn1_end_tag(data)) goto prot_err; break; } case ASN1_APPLICATION_SIMPLE(LDAP_TAG_AbandonRequest): { struct ldap_AbandonRequest *r = &msg->r.AbandonRequest; msg->type = LDAP_TAG_AbandonRequest; - asn1_start_tag(data, tag); - asn1_read_implicit_Integer(data, &r->messageid); - asn1_end_tag(data); + if (!asn1_start_tag(data, tag)) goto prot_err; + if (!asn1_read_implicit_Integer(data, &r->messageid)) goto prot_err; + if (!asn1_end_tag(data)) goto prot_err; break; } @@ -1482,9 +1484,9 @@ _PUBLIC_ NTSTATUS ldap_decode(struct asn1_data *data, DATA_BLOB tmp_blob = data_blob(NULL, 0); msg->type = LDAP_TAG_ExtendedRequest; - asn1_start_tag(data,tag); + if (!asn1_start_tag(data,tag)) goto prot_err; if (!asn1_read_ContextSimple(data, 0, &tmp_blob)) { - return NT_STATUS_LDAP(LDAP_PROTOCOL_ERROR); + goto prot_err; } r->oid = blob2string_talloc(msg, tmp_blob); data_blob_free(&tmp_blob); @@ -1493,7 +1495,7 @@ _PUBLIC_ NTSTATUS ldap_decode(struct asn1_data *data, } if (asn1_peek_tag(data, ASN1_CONTEXT_SIMPLE(1))) { - asn1_read_ContextSimple(data, 1, &tmp_blob); + if (!asn1_read_ContextSimple(data, 1, &tmp_blob)) goto prot_err; r->value = talloc(msg, DATA_BLOB); if (!r->value) { return NT_STATUS_LDAP(LDAP_OPERATIONS_ERROR); @@ -1504,7 +1506,7 @@ _PUBLIC_ NTSTATUS ldap_decode(struct asn1_data *data, r->value = NULL; } - asn1_end_tag(data); + if (!asn1_end_tag(data)) goto prot_err; break; } @@ -1513,11 +1515,11 @@ _PUBLIC_ NTSTATUS ldap_decode(struct asn1_data *data, DATA_BLOB tmp_blob = data_blob(NULL, 0); msg->type = LDAP_TAG_ExtendedResponse; - asn1_start_tag(data, tag); - ldap_decode_response(msg, data, &r->response); + if (!asn1_start_tag(data, tag)) goto prot_err; + if (!ldap_decode_response(msg, data, &r->response)) goto prot_err; if (asn1_peek_tag(data, ASN1_CONTEXT_SIMPLE(10))) { - asn1_read_ContextSimple(data, 1, &tmp_blob); + if (!asn1_read_ContextSimple(data, 1, &tmp_blob)) goto prot_err; r->oid = blob2string_talloc(msg, tmp_blob); data_blob_free(&tmp_blob); if (!r->oid) { @@ -1528,7 +1530,7 @@ _PUBLIC_ NTSTATUS ldap_decode(struct asn1_data *data, } if (asn1_peek_tag(data, ASN1_CONTEXT_SIMPLE(11))) { - asn1_read_ContextSimple(data, 1, &tmp_blob); + if (!asn1_read_ContextSimple(data, 1, &tmp_blob)) goto prot_err; r->value = talloc(msg, DATA_BLOB); if (!r->value) { return NT_STATUS_LDAP(LDAP_OPERATIONS_ERROR); @@ -1539,11 +1541,11 @@ _PUBLIC_ NTSTATUS ldap_decode(struct asn1_data *data, r->value = NULL; } - asn1_end_tag(data); + if (!asn1_end_tag(data)) goto prot_err; break; } - default: - return NT_STATUS_LDAP(LDAP_PROTOCOL_ERROR); + default: + goto prot_err; } msg->controls = NULL; @@ -1554,7 +1556,7 @@ _PUBLIC_ NTSTATUS ldap_decode(struct asn1_data *data, struct ldb_control **ctrl = NULL; bool *decoded = NULL; - asn1_start_tag(data, ASN1_CONTEXT(0)); + if (!asn1_start_tag(data, ASN1_CONTEXT(0))) goto prot_err; while (asn1_peek_tag(data, ASN1_SEQUENCE(0))) { DATA_BLOB value; @@ -1576,9 +1578,9 @@ _PUBLIC_ NTSTATUS ldap_decode(struct asn1_data *data, } if (!ldap_decode_control_wrapper(ctrl[i], data, ctrl[i], &value)) { - return NT_STATUS_LDAP(LDAP_PROTOCOL_ERROR); + goto prot_err; } - + if (!ldap_decode_control_value(ctrl[i], value, control_handlers, ctrl[i])) { @@ -1603,14 +1605,18 @@ _PUBLIC_ NTSTATUS ldap_decode(struct asn1_data *data, msg->controls = ctrl; msg->controls_decoded = decoded; - asn1_end_tag(data); + if (!asn1_end_tag(data)) goto prot_err; } - asn1_end_tag(data); + if (!asn1_end_tag(data)) goto prot_err; if ((data->has_error) || (data->nesting != NULL)) { return NT_STATUS_LDAP(LDAP_PROTOCOL_ERROR); } return NT_STATUS_OK; + + prot_err: + + return NT_STATUS_LDAP(LDAP_PROTOCOL_ERROR); } diff --git a/libcli/ldap/ldap_message.h b/libcli/ldap/ldap_message.h index b41f7f5..2f64881 100644 --- a/libcli/ldap/ldap_message.h +++ b/libcli/ldap/ldap_message.h @@ -228,7 +228,7 @@ bool asn1_read_OctetString_talloc(TALLOC_CTX *mem_ctx, struct asn1_data *data, const char **result); -void ldap_decode_attribs_bare(TALLOC_CTX *mem_ctx, struct asn1_data *data, +bool ldap_decode_attribs_bare(TALLOC_CTX *mem_ctx, struct asn1_data *data, struct ldb_message_element **attributes, int *num_attributes); diff --git a/source4/libcli/ldap/ldap_controls.c b/source4/libcli/ldap/ldap_controls.c index 17d96f6..983082b 100644 --- a/source4/libcli/ldap/ldap_controls.c +++ b/source4/libcli/ldap/ldap_controls.c @@ -1187,10 +1187,10 @@ static bool decode_openldap_dereference(void *mem_ctx, DATA_BLOB in, void *_out) if (!asn1_start_tag(data, ASN1_CONTEXT(0))) { return false; } - - ldap_decode_attribs_bare(r, data, &r[i]->attributes, - &r[i]->num_attributes); - + if (!ldap_decode_attribs_bare(r, data, &r[i]->attributes, + &r[i]->num_attributes)) { + return false; + } if (!asn1_end_tag(data)) { return false; } -- 1.9.1 From 9d989c9dd7a5b92d0c5d65287935471b83b6e884 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 18 Sep 2014 13:58:45 -0700 Subject: [PATCH 11/17] CVE-2015-7540: lib: util: Check *every* asn1 return call and early return. BUG: https://bugzilla.samba.org/show_bug.cgi?id=9187 Signed-off-by: Jeremy Allison Reviewed-by: Volker Lendecke Autobuild-User(master): Jeremy Allison Autobuild-Date(master): Fri Sep 19 01:29:00 CEST 2014 on sn-devel-104 (cherry picked from commit b9d3fd4cc551df78a7b066ee8ce43bbaa3ff994a) --- lib/util/asn1.c | 106 ++++++++++++++++++++++++-------------------------------- 1 file changed, 45 insertions(+), 61 deletions(-) diff --git a/lib/util/asn1.c b/lib/util/asn1.c index 70637a3..ec29450 100644 --- a/lib/util/asn1.c +++ b/lib/util/asn1.c @@ -326,87 +326,76 @@ bool asn1_write_OID(struct asn1_data *data, const char *OID) /* write an octet string */ bool asn1_write_OctetString(struct asn1_data *data, const void *p, size_t length) { - asn1_push_tag(data, ASN1_OCTET_STRING); - asn1_write(data, p, length); - asn1_pop_tag(data); - return !data->has_error; + if (!asn1_push_tag(data, ASN1_OCTET_STRING)) return false; + if (!asn1_write(data, p, length)) return false; + return asn1_pop_tag(data); } /* write a LDAP string */ bool asn1_write_LDAPString(struct asn1_data *data, const char *s) { - asn1_write(data, s, strlen(s)); - return !data->has_error; + return asn1_write(data, s, strlen(s)); } /* write a LDAP string from a DATA_BLOB */ bool asn1_write_DATA_BLOB_LDAPString(struct asn1_data *data, const DATA_BLOB *s) { - asn1_write(data, s->data, s->length); - return !data->has_error; + return asn1_write(data, s->data, s->length); } /* write a general string */ bool asn1_write_GeneralString(struct asn1_data *data, const char *s) { - asn1_push_tag(data, ASN1_GENERAL_STRING); - asn1_write_LDAPString(data, s); - asn1_pop_tag(data); - return !data->has_error; + if (!asn1_push_tag(data, ASN1_GENERAL_STRING)) return false; + if (!asn1_write_LDAPString(data, s)) return false; + return asn1_pop_tag(data); } bool asn1_write_ContextSimple(struct asn1_data *data, uint8_t num, DATA_BLOB *blob) { - asn1_push_tag(data, ASN1_CONTEXT_SIMPLE(num)); - asn1_write(data, blob->data, blob->length); - asn1_pop_tag(data); - return !data->has_error; + if (!asn1_push_tag(data, ASN1_CONTEXT_SIMPLE(num))) return false; + if (!asn1_write(data, blob->data, blob->length)) return false; + return asn1_pop_tag(data); } /* write a BOOLEAN */ bool asn1_write_BOOLEAN(struct asn1_data *data, bool v) { - asn1_push_tag(data, ASN1_BOOLEAN); - asn1_write_uint8(data, v ? 0xFF : 0); - asn1_pop_tag(data); - return !data->has_error; + if (!asn1_push_tag(data, ASN1_BOOLEAN)) return false; + if (!asn1_write_uint8(data, v ? 0xFF : 0)) return false; + return asn1_pop_tag(data); } bool asn1_read_BOOLEAN(struct asn1_data *data, bool *v) { uint8_t tmp = 0; - asn1_start_tag(data, ASN1_BOOLEAN); - asn1_read_uint8(data, &tmp); + if (!asn1_start_tag(data, ASN1_BOOLEAN)) return false; + *v = false; + if (!asn1_read_uint8(data, &tmp)) return false; if (tmp == 0xFF) { *v = true; - } else { - *v = false; } - asn1_end_tag(data); - return !data->has_error; + return asn1_end_tag(data); } /* write a BOOLEAN in a simple context */ bool asn1_write_BOOLEAN_context(struct asn1_data *data, bool v, int context) { - asn1_push_tag(data, ASN1_CONTEXT_SIMPLE(context)); - asn1_write_uint8(data, v ? 0xFF : 0); - asn1_pop_tag(data); - return !data->has_error; + if (!asn1_push_tag(data, ASN1_CONTEXT_SIMPLE(context))) return false; + if (!asn1_write_uint8(data, v ? 0xFF : 0)) return false; + return asn1_pop_tag(data); } bool asn1_read_BOOLEAN_context(struct asn1_data *data, bool *v, int context) { uint8_t tmp = 0; - asn1_start_tag(data, ASN1_CONTEXT_SIMPLE(context)); - asn1_read_uint8(data, &tmp); + if (!asn1_start_tag(data, ASN1_CONTEXT_SIMPLE(context))) return false; + *v = false; + if (!asn1_read_uint8(data, &tmp)) return false; if (tmp == 0xFF) { *v = true; - } else { - *v = false; } - asn1_end_tag(data); - return !data->has_error; + return asn1_end_tag(data); } /* check a BOOLEAN */ @@ -414,12 +403,12 @@ bool asn1_check_BOOLEAN(struct asn1_data *data, bool v) { uint8_t b = 0; - asn1_read_uint8(data, &b); + if (!asn1_read_uint8(data, &b)) return false; if (b != ASN1_BOOLEAN) { data->has_error = true; return false; } - asn1_read_uint8(data, &b); + if (!asn1_read_uint8(data, &b)) return false; if (b != v) { data->has_error = true; return false; @@ -770,9 +759,8 @@ bool asn1_read_OID(struct asn1_data *data, TALLOC_CTX *mem_ctx, char **OID) return false; } - asn1_read(data, blob.data, len); - asn1_end_tag(data); - if (data->has_error) { + if (!asn1_read(data, blob.data, len)) return false; + if (!asn1_end_tag(data)) { data_blob_free(&blob); return false; } @@ -817,9 +805,8 @@ bool asn1_read_LDAPString(struct asn1_data *data, TALLOC_CTX *mem_ctx, char **s) data->has_error = true; return false; } - asn1_read(data, *s, len); (*s)[len] = 0; - return !data->has_error; + return asn1_read(data, *s, len); } @@ -848,17 +835,17 @@ bool asn1_read_OctetString(struct asn1_data *data, TALLOC_CTX *mem_ctx, DATA_BLO data->has_error = true; return false; } - asn1_read(data, blob->data, len); - asn1_end_tag(data); + if (!asn1_read(data, blob->data, len)) goto err; + if (!asn1_end_tag(data)) goto err; blob->length--; blob->data[len] = 0; - - if (data->has_error) { - data_blob_free(blob); - *blob = data_blob_null; - return false; - } return true; + + err: + + data_blob_free(blob); + *blob = data_blob_null; + return false; } bool asn1_read_ContextSimple(struct asn1_data *data, uint8_t num, DATA_BLOB *blob) @@ -876,9 +863,8 @@ bool asn1_read_ContextSimple(struct asn1_data *data, uint8_t num, DATA_BLOB *blo data->has_error = true; return false; } - asn1_read(data, blob->data, len); - asn1_end_tag(data); - return !data->has_error; + if (!asn1_read(data, blob->data, len)) return false; + return asn1_end_tag(data); } /* read an integer without tag*/ @@ -966,8 +952,8 @@ bool asn1_check_enumerated(struct asn1_data *data, int v) { uint8_t b; if (!asn1_start_tag(data, ASN1_ENUMERATED)) return false; - asn1_read_uint8(data, &b); - asn1_end_tag(data); + if (!asn1_read_uint8(data, &b)) return false; + if (!asn1_end_tag(data)) return false; if (v != b) data->has_error = false; @@ -979,9 +965,8 @@ bool asn1_check_enumerated(struct asn1_data *data, int v) bool asn1_write_enumerated(struct asn1_data *data, uint8_t v) { if (!asn1_push_tag(data, ASN1_ENUMERATED)) return false; - asn1_write_uint8(data, v); - asn1_pop_tag(data); - return !data->has_error; + if (!asn1_write_uint8(data, v)) return false; + return asn1_pop_tag(data); } /* @@ -1022,8 +1007,7 @@ NTSTATUS asn1_full_tag(DATA_BLOB blob, uint8_t tag, size_t *packet_size) asn1->data = blob.data; asn1->length = blob.length; - asn1_start_tag(asn1, tag); - if (asn1->has_error) { + if (!asn1_start_tag(asn1, tag)) { talloc_free(asn1); return STATUS_MORE_ENTRIES; } -- 1.9.1 From f0cb216f6385460d4d3c728257baaaa26a95c5d1 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 9 Jul 2015 10:58:11 -0700 Subject: [PATCH 12/17] CVE-2015-5252: s3: smbd: Fix symlink verification (file access outside the share). Ensure matching component ends in '/' or '\0'. BUG: https://bugzilla.samba.org/show_bug.cgi?id=11395 Signed-off-by: Jeremy Allison Reviewed-by: Volker Lendecke --- source3/smbd/vfs.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/source3/smbd/vfs.c b/source3/smbd/vfs.c index 118f6c9..edf4ee1 100644 --- a/source3/smbd/vfs.c +++ b/source3/smbd/vfs.c @@ -976,6 +976,7 @@ NTSTATUS check_reduced_name_with_privilege(connection_struct *conn, struct smb_filename *smb_fname_cwd = NULL; struct privilege_paths *priv_paths = NULL; int ret; + bool matched; DEBUG(3,("check_reduced_name_with_privilege [%s] [%s]\n", fname, @@ -1070,7 +1071,10 @@ NTSTATUS check_reduced_name_with_privilege(connection_struct *conn, } rootdir_len = strlen(conn_rootdir); - if (strncmp(conn_rootdir, resolved_name, rootdir_len) != 0) { + matched = (strncmp(conn_rootdir, resolved_name, rootdir_len) == 0); + + if (!matched || (resolved_name[rootdir_len] != '/' && + resolved_name[rootdir_len] != '\0')) { DEBUG(2, ("check_reduced_name_with_privilege: Bad access " "attempt: %s is a symlink outside the " "share path\n", @@ -1210,6 +1214,7 @@ NTSTATUS check_reduced_name(connection_struct *conn, const char *fname) if (!allow_widelinks || !allow_symlinks) { const char *conn_rootdir; size_t rootdir_len; + bool matched; conn_rootdir = SMB_VFS_CONNECTPATH(conn, fname); if (conn_rootdir == NULL) { @@ -1220,8 +1225,10 @@ NTSTATUS check_reduced_name(connection_struct *conn, const char *fname) } rootdir_len = strlen(conn_rootdir); - if (strncmp(conn_rootdir, resolved_name, - rootdir_len) != 0) { + matched = (strncmp(conn_rootdir, resolved_name, + rootdir_len) == 0); + if (!matched || (resolved_name[rootdir_len] != '/' && + resolved_name[rootdir_len] != '\0')) { DEBUG(2, ("check_reduced_name: Bad access " "attempt: %s is a symlink outside the " "share path\n", fname)); -- 1.9.1 From fa777786d75272e3190dcbd32eeff9b3e4f03bde Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 23 Oct 2015 14:54:31 -0700 Subject: [PATCH 13/17] CVE-2015-5299: s3-shadow-copy2: fix missing access check on snapdir Fix originally from https://bugzilla.samba.org/show_bug.cgi?id=11529 Signed-off-by: Jeremy Allison Reviewed-by: David Disseldorp --- source3/modules/vfs_shadow_copy2.c | 45 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/source3/modules/vfs_shadow_copy2.c b/source3/modules/vfs_shadow_copy2.c index fca05cf..07e2f8a 100644 --- a/source3/modules/vfs_shadow_copy2.c +++ b/source3/modules/vfs_shadow_copy2.c @@ -30,6 +30,7 @@ */ #include "includes.h" +#include "smbd/smbd.h" #include "system/filesys.h" #include "include/ntioctl.h" #include @@ -1138,6 +1139,42 @@ static char *have_snapdir(struct vfs_handle_struct *handle, return NULL; } +static bool check_access_snapdir(struct vfs_handle_struct *handle, + const char *path) +{ + struct smb_filename smb_fname; + int ret; + NTSTATUS status; + + ZERO_STRUCT(smb_fname); + smb_fname.base_name = talloc_asprintf(talloc_tos(), + "%s", + path); + if (smb_fname.base_name == NULL) { + return false; + } + + ret = SMB_VFS_NEXT_STAT(handle, &smb_fname); + if (ret != 0 || !S_ISDIR(smb_fname.st.st_ex_mode)) { + TALLOC_FREE(smb_fname.base_name); + return false; + } + + status = smbd_check_access_rights(handle->conn, + &smb_fname, + false, + SEC_DIR_LIST); + if (!NT_STATUS_IS_OK(status)) { + DEBUG(0,("user does not have list permission " + "on snapdir %s\n", + smb_fname.base_name)); + TALLOC_FREE(smb_fname.base_name); + return false; + } + TALLOC_FREE(smb_fname.base_name); + return true; +} + /** * Find the snapshot directory (if any) for the given * filename (which is relative to the share). @@ -1287,6 +1324,7 @@ static int shadow_copy2_get_shadow_copy_data( const char *snapdir; struct dirent *d; TALLOC_CTX *tmp_ctx = talloc_stackframe(); + bool ret; snapdir = shadow_copy2_find_snapdir(tmp_ctx, handle, fsp->fsp_name); if (snapdir == NULL) { @@ -1296,6 +1334,13 @@ static int shadow_copy2_get_shadow_copy_data( talloc_free(tmp_ctx); return -1; } + ret = check_access_snapdir(handle, snapdir); + if (!ret) { + DEBUG(0,("access denied on listing snapdir %s\n", snapdir)); + errno = EACCES; + talloc_free(tmp_ctx); + return -1; + } p = SMB_VFS_NEXT_OPENDIR(handle, snapdir, NULL, 0); -- 1.9.1 From d9e943e351a752ba627314da7fb8d2f6f1eb44b3 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 30 Sep 2015 21:17:02 +0200 Subject: [PATCH 14/17] CVE-2015-5296: s3:libsmb: force signing when requiring encryption in do_connect() BUG: https://bugzilla.samba.org/show_bug.cgi?id=11536 Signed-off-by: Stefan Metzmacher Reviewed-by: Jeremy Allison --- source3/libsmb/clidfs.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/source3/libsmb/clidfs.c b/source3/libsmb/clidfs.c index 729f4fe..c72cbfd 100644 --- a/source3/libsmb/clidfs.c +++ b/source3/libsmb/clidfs.c @@ -114,6 +114,11 @@ static NTSTATUS do_connect(TALLOC_CTX *ctx, const char *domain; NTSTATUS status; int flags = 0; + int signing_state = get_cmdline_auth_info_signing_state(auth_info); + + if (force_encrypt) { + signing_state = SMB_SIGNING_REQUIRED; + } /* make a copy so we don't modify the global string 'service' */ servicename = talloc_strdup(ctx,share); @@ -152,7 +157,7 @@ static NTSTATUS do_connect(TALLOC_CTX *ctx, status = cli_connect_nb( server, NULL, port, name_type, NULL, - get_cmdline_auth_info_signing_state(auth_info), + signing_state, flags, &c); if (!NT_STATUS_IS_OK(status)) { -- 1.9.1 From 4c3a492259ceefe3d02df690d4369291627883a2 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 30 Sep 2015 21:17:02 +0200 Subject: [PATCH 15/17] CVE-2015-5296: s3:libsmb: force signing when requiring encryption in SMBC_server_internal() BUG: https://bugzilla.samba.org/show_bug.cgi?id=11536 Signed-off-by: Stefan Metzmacher Reviewed-by: Jeremy Allison --- source3/libsmb/libsmb_server.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/source3/libsmb/libsmb_server.c b/source3/libsmb/libsmb_server.c index b1a4ed7..ea81f84 100644 --- a/source3/libsmb/libsmb_server.c +++ b/source3/libsmb/libsmb_server.c @@ -273,6 +273,7 @@ SMBC_server_internal(TALLOC_CTX *ctx, char *newserver, *newshare; int flags = 0; struct smbXcli_tcon *tcon = NULL; + int signing_state = SMB_SIGNING_DEFAULT; ZERO_STRUCT(c); *in_cache = false; @@ -439,6 +440,10 @@ SMBC_server_internal(TALLOC_CTX *ctx, flags |= CLI_FULL_CONNECTION_USE_NT_HASH; } + if (context->internal->smb_encryption_level != SMBC_ENCRYPTLEVEL_NONE) { + signing_state = SMB_SIGNING_REQUIRED; + } + if (port == 0) { if (share == NULL || *share == '\0' || is_ipc) { /* @@ -446,7 +451,7 @@ SMBC_server_internal(TALLOC_CTX *ctx, */ status = cli_connect_nb(server_n, NULL, NBT_SMB_PORT, 0x20, smbc_getNetbiosName(context), - SMB_SIGNING_DEFAULT, flags, &c); + signing_state, flags, &c); } } @@ -456,7 +461,7 @@ SMBC_server_internal(TALLOC_CTX *ctx, */ status = cli_connect_nb(server_n, NULL, port, 0x20, smbc_getNetbiosName(context), - SMB_SIGNING_DEFAULT, flags, &c); + signing_state, flags, &c); } if (!NT_STATUS_IS_OK(status)) { @@ -745,6 +750,7 @@ SMBC_attr_server(TALLOC_CTX *ctx, ipc_srv = SMBC_find_server(ctx, context, server, "*IPC$", pp_workgroup, pp_username, pp_password); if (!ipc_srv) { + int signing_state = SMB_SIGNING_DEFAULT; /* We didn't find a cached connection. Get the password */ if (!*pp_password || (*pp_password)[0] == '\0') { @@ -766,6 +772,9 @@ SMBC_attr_server(TALLOC_CTX *ctx, if (smbc_getOptionUseCCache(context)) { flags |= CLI_FULL_CONNECTION_USE_CCACHE; } + if (context->internal->smb_encryption_level != SMBC_ENCRYPTLEVEL_NONE) { + signing_state = SMB_SIGNING_REQUIRED; + } nt_status = cli_full_connection(&ipc_cli, lp_netbios_name(), server, @@ -774,7 +783,7 @@ SMBC_attr_server(TALLOC_CTX *ctx, *pp_workgroup, *pp_password, flags, - SMB_SIGNING_DEFAULT); + signing_state); if (! NT_STATUS_IS_OK(nt_status)) { DEBUG(1,("cli_full_connection failed! (%s)\n", nt_errstr(nt_status))); -- 1.9.1 From c634a143a876bd5a724d830c54fe12ef6d68d5fd Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 30 Sep 2015 21:23:25 +0200 Subject: [PATCH 16/17] CVE-2015-5296: libcli/smb: make sure we require signing when we demand encryption on a session BUG: https://bugzilla.samba.org/show_bug.cgi?id=11536 Signed-off-by: Stefan Metzmacher Reviewed-by: Jeremy Allison --- libcli/smb/smbXcli_base.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/libcli/smb/smbXcli_base.c b/libcli/smb/smbXcli_base.c index 5063e59..546ce40 100644 --- a/libcli/smb/smbXcli_base.c +++ b/libcli/smb/smbXcli_base.c @@ -4754,6 +4754,9 @@ uint8_t smb2cli_session_security_mode(struct smbXcli_session *session) if (conn->mandatory_signing) { security_mode |= SMB2_NEGOTIATE_SIGNING_REQUIRED; } + if (session->smb2->should_sign) { + security_mode |= SMB2_NEGOTIATE_SIGNING_REQUIRED; + } return security_mode; } @@ -5031,6 +5034,14 @@ NTSTATUS smb2cli_session_set_channel_key(struct smbXcli_session *session, NTSTATUS smb2cli_session_encryption_on(struct smbXcli_session *session) { + if (!session->smb2->should_sign) { + /* + * We need required signing on the session + * in order to prevent man in the middle attacks. + */ + return NT_STATUS_INVALID_PARAMETER_MIX; + } + if (session->smb2->should_encrypt) { return NT_STATUS_OK; } -- 1.9.1 From bf13cbd3f33c31483b172fc094b0e5946e899bc4 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Wed, 18 Nov 2015 17:36:21 +1300 Subject: [PATCH 17/17] CVE-2015-8467: samdb: Match MS15-096 behaviour for userAccountControl Swapping between account types is now restricted Bug: https://bugzilla.samba.org/show_bug.cgi?id=11552 Signed-off-by: Andrew Bartlett Reviewed-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source4/dsdb/samdb/ldb_modules/samldb.c | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/samldb.c b/source4/dsdb/samdb/ldb_modules/samldb.c index 2c8a064..97934c7 100644 --- a/source4/dsdb/samdb/ldb_modules/samldb.c +++ b/source4/dsdb/samdb/ldb_modules/samldb.c @@ -1468,12 +1468,15 @@ static int samldb_check_user_account_control_acl(struct samldb_ctx *ac, struct security_token *user_token; struct security_descriptor *domain_sd; struct ldb_dn *domain_dn = ldb_get_default_basedn(ldb_module_get_ctx(ac->module)); + struct ldb_context *ldb = ldb_module_get_ctx(ac->module); const struct uac_to_guid { uint32_t uac; + uint32_t priv_to_change_from; const char *oid; const char *guid; enum sec_privilege privilege; bool delete_is_privileged; + bool admin_required; const char *error_string; } map[] = { { @@ -1502,6 +1505,16 @@ static int samldb_check_user_account_control_acl(struct samldb_ctx *ac, .error_string = "Adding the UF_PARTIAL_SECRETS_ACCOUNT bit in userAccountControl requires the DS-Install-Replica right that was not given on the Domain object" }, { + .uac = UF_WORKSTATION_TRUST_ACCOUNT, + .priv_to_change_from = UF_NORMAL_ACCOUNT, + .error_string = "Swapping UF_NORMAL_ACCOUNT to UF_WORKSTATION_TRUST_ACCOUNT requires the user to be a member of the domain admins group" + }, + { + .uac = UF_NORMAL_ACCOUNT, + .priv_to_change_from = UF_WORKSTATION_TRUST_ACCOUNT, + .error_string = "Swapping UF_WORKSTATION_TRUST_ACCOUNT to UF_NORMAL_ACCOUNT requires the user to be a member of the domain admins group" + }, + { .uac = UF_INTERDOMAIN_TRUST_ACCOUNT, .oid = DSDB_CONTROL_PERMIT_INTERDOMAIN_TRUST_UAC_OID, .error_string = "Updating the UF_INTERDOMAIN_TRUST_ACCOUNT bit in userAccountControl is not permitted over LDAP. This bit is restricted to the LSA CreateTrustedDomain interface", @@ -1553,7 +1566,7 @@ static int samldb_check_user_account_control_acl(struct samldb_ctx *ac, return ldb_module_operr(ac->module); } - ret = dsdb_get_sd_from_ldb_message(ldb_module_get_ctx(ac->module), + ret = dsdb_get_sd_from_ldb_message(ldb, ac, res->msgs[0], &domain_sd); if (ret != LDB_SUCCESS) { @@ -1580,12 +1593,19 @@ static int samldb_check_user_account_control_acl(struct samldb_ctx *ac, if (have_priv == false) { ret = LDB_ERR_INSUFFICIENT_ACCESS_RIGHTS; } - } else { + } else if (map[i].priv_to_change_from & user_account_control_old) { + bool is_admin = security_token_has_builtin_administrators(user_token); + if (is_admin == false) { + ret = LDB_ERR_INSUFFICIENT_ACCESS_RIGHTS; + } + } else if (map[i].guid) { ret = acl_check_extended_right(ac, domain_sd, user_token, map[i].guid, SEC_ADS_CONTROL_ACCESS, sid); + } else { + ret = LDB_SUCCESS; } if (ret != LDB_SUCCESS) { break; -- 1.9.1