From 9c7e988969f5d80454d62c1d2154810b3976b818 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 9 Jun 2015 12:42:10 -0700 Subject: [PATCH 01/15] 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 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) ) { -- 1.9.1 From 3c68b500a7beca60559dc073f53cc1aa1f9a8ba1 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 9 Jun 2015 14:00:01 -0700 Subject: [PATCH 02/15] 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 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); -- 1.9.1 From 813eceafcada3a2ce260499d8792a943426bdac9 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Tue, 24 Nov 2015 13:07:23 +1300 Subject: [PATCH 03/15] 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 06f2d959478c20cdfe33361e727c1ce055cc62cf Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Tue, 24 Nov 2015 13:09:36 +1300 Subject: [PATCH 04/15] 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 405170bd75c38b34c2dd635e7dc3ed90225a1776 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Tue, 24 Nov 2015 13:47:16 +1300 Subject: [PATCH 05/15] 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 9c068332f0dd03d7cc00fadc50a5707d4d53a09f Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Tue, 24 Nov 2015 13:49:09 +1300 Subject: [PATCH 06/15] 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 75b3ce698912fa15a63078ed6325d50caec3717b Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Tue, 24 Nov 2015 13:54:09 +1300 Subject: [PATCH 07/15] 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 aa68bd3430c421b83b24135d6ea4e7b6cd47d79f Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Thu, 26 Nov 2015 11:17:11 +1300 Subject: [PATCH 08/15] 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 6dc18a68a0581c33eb9005ebca84631fff176975 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Tue, 8 Dec 2015 10:55:42 +0100 Subject: [PATCH 09/15] 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.20 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 6391e74..18e315b 100755 --- a/lib/ldb/wscript +++ b/lib/ldb/wscript @@ -2,6 +2,7 @@ APPNAME = 'ldb' VERSION = '1.1.20' +SYSTEM_VERSION = '1.1.24' blddir = 'bin' @@ -55,11 +56,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 2b25a10..9433a0d 100755 --- a/script/autobuild.py +++ b/script/autobuild.py @@ -93,7 +93,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.20,pyldb-util:1.1.20 --abi-check --enable-debug -C ${PREFIX}", "text/plain"), ("make", "make", "text/plain"), ("install", "make install", "text/plain"), ("dist", "make dist", "text/plain")], -- 1.9.1 From 79e5023a77b851b60a3a3e723013539f1e39b99b Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 9 Jul 2015 10:58:11 -0700 Subject: [PATCH 10/15] 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 1281322..7138759 100644 --- a/source3/smbd/vfs.c +++ b/source3/smbd/vfs.c @@ -996,6 +996,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, @@ -1090,7 +1091,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", @@ -1230,6 +1234,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) { @@ -1240,8 +1245,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 1d8efe6abf1c98f62f07c4c4b869d8169d6904b4 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 23 Oct 2015 14:54:31 -0700 Subject: [PATCH 11/15] 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 439df5d..c5c2015 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 @@ -1179,6 +1180,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). @@ -1328,6 +1365,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) { @@ -1337,6 +1375,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 05d09fb2415f386ce9f2a3f4a86d10ef1abca020 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 30 Sep 2015 21:17:02 +0200 Subject: [PATCH 12/15] 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 b823370..5dfddee 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 3e8f1123b2f89951b498d3d9a9af7f8dd68038c9 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 30 Sep 2015 21:17:02 +0200 Subject: [PATCH 13/15] 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 5410099..0a58d8c 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 41e1e8b9a25ef1052258f4355e2d2c2f41e29b14 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 30 Sep 2015 21:23:25 +0200 Subject: [PATCH 14/15] 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 69599bd..b00afbc 100644 --- a/libcli/smb/smbXcli_base.c +++ b/libcli/smb/smbXcli_base.c @@ -5085,6 +5085,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; } @@ -5383,6 +5386,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 2483d66af2a298e1722dbe45ccadddf609817d67 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Wed, 18 Nov 2015 17:36:21 +1300 Subject: [PATCH 15/15] 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 54e2e5e..5537fdf 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