[SCM] Samba Shared Repository - branch master updated

Karolin Seeger kseeger at samba.org
Tue Oct 29 11:59:04 UTC 2019


The branch, master has been updated
       via  ef58222616f CVE-2019-14833 dsdb: send full password to check password script
       via  d524c7ddee9 CVE-2019-14833: Use utf8 characters in the unacceptable password
       via  7ccc302b4bb CVE-2019-10218 - s3: libsmb: Protect SMB2 client code from evil server returned names.
       via  9f7a622b2bd CVE-2019-10218 - s3: libsmb: Protect SMB1 client code from evil server returned names.
      from  2669cecc51f libnet_join: add SPNs for additional-dns-hostnames entries

https://git.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit ef58222616fc3175f189417ce878d8413ba2d294
Author: Björn Baumbach <bb at sernet.de>
Date:   Tue Aug 6 16:32:32 2019 +0200

    CVE-2019-14833 dsdb: send full password to check password script
    
    utf8_len represents the number of characters (not bytes) of the
    password. If the password includes multi-byte characters it is required
    to write the total number of bytes to the check password script.
    Otherwise the last bytes of the password string would be ignored.
    
    Therefore we rename utf8_len to be clear what it does and does
    not represent.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=12438
    
    Signed-off-by: Björn Baumbach <bb at sernet.de>
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    
    Autobuild-User(master): Karolin Seeger <kseeger at samba.org>
    Autobuild-Date(master): Tue Oct 29 11:58:45 UTC 2019 on sn-devel-184

commit d524c7ddee92a457ba680853b6c25c877d881ff8
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Thu Sep 19 11:50:01 2019 +1200

    CVE-2019-14833: Use utf8 characters in the unacceptable password
    
    This shows that the "check password script" handling has a bug.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=12438
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>

commit 7ccc302b4bb9e0a9b695074959dc45e6fc4902bb
Author: Jeremy Allison <jra at samba.org>
Date:   Tue Aug 6 12:08:09 2019 -0700

    CVE-2019-10218 - s3: libsmb: Protect SMB2 client code from evil server returned names.
    
    Disconnect with NT_STATUS_INVALID_NETWORK_RESPONSE if so.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14071
    
    Signed-off-by: Jeremy Allison <jra at samba.org>

commit 9f7a622b2bd4a42fad3e669e83fe07c5d7115dc6
Author: Jeremy Allison <jra at samba.org>
Date:   Mon Aug 5 13:39:53 2019 -0700

    CVE-2019-10218 - s3: libsmb: Protect SMB1 client code from evil server returned names.
    
    Disconnect with NT_STATUS_INVALID_NETWORK_RESPONSE if so.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14071
    
    Signed-off-by: Jeremy Allison <jra at samba.org>

-----------------------------------------------------------------------

Summary of changes:
 selftest/target/Samba4.pm      |  2 +-
 source3/libsmb/cli_smb2_fnum.c |  7 ++++
 source3/libsmb/clilist.c       | 75 ++++++++++++++++++++++++++++++++++++++++++
 source3/libsmb/proto.h         |  3 ++
 source4/dsdb/common/util.c     | 30 +++++++++++++----
 5 files changed, 110 insertions(+), 7 deletions(-)


Changeset truncated at 500 lines:

diff --git a/selftest/target/Samba4.pm b/selftest/target/Samba4.pm
index 9df9e84ff63..1310e2ff09f 100755
--- a/selftest/target/Samba4.pm
+++ b/selftest/target/Samba4.pm
@@ -1882,7 +1882,7 @@ sub provision_chgdcpass($$)
 	print "PROVISIONING CHGDCPASS...\n";
 	# This environment disallows the use of this password
 	# (and also removes the default AD complexity checks)
-	my $unacceptable_password = "widk3Dsle32jxdBdskldsk55klASKQ";
+	my $unacceptable_password = "Paßßword-widk3Dsle32jxdBdskldsk55klASKQ";
 	my $extra_smb_conf = "
 	check password script = $self->{srcdir}/selftest/checkpassword_arg1.sh ${unacceptable_password}
 	allow dcerpc auth level connect:lsarpc = yes
diff --git a/source3/libsmb/cli_smb2_fnum.c b/source3/libsmb/cli_smb2_fnum.c
index 15f1420dd8f..4cae87853db 100644
--- a/source3/libsmb/cli_smb2_fnum.c
+++ b/source3/libsmb/cli_smb2_fnum.c
@@ -1442,6 +1442,13 @@ NTSTATUS cli_smb2_list(struct cli_state *cli,
 				goto fail;
 			}
 
+			/* Protect against server attack. */
+			status = is_bad_finfo_name(cli, finfo);
+			if (!NT_STATUS_IS_OK(status)) {
+				smbXcli_conn_disconnect(cli->conn, status);
+				goto fail;
+			}
+
 			if (dir_check_ftype((uint32_t)finfo->mode,
 					(uint32_t)attribute)) {
 				/*
diff --git a/source3/libsmb/clilist.c b/source3/libsmb/clilist.c
index 58dac65f4c7..f868e72a239 100644
--- a/source3/libsmb/clilist.c
+++ b/source3/libsmb/clilist.c
@@ -24,6 +24,66 @@
 #include "trans2.h"
 #include "../libcli/smb/smbXcli_base.h"
 
+/****************************************************************************
+ Check if a returned directory name is safe.
+****************************************************************************/
+
+static NTSTATUS is_bad_name(bool windows_names, const char *name)
+{
+	const char *bad_name_p = NULL;
+
+	bad_name_p = strchr(name, '/');
+	if (bad_name_p != NULL) {
+		/*
+		 * Windows and POSIX names can't have '/'.
+		 * Server is attacking us.
+		 */
+		return NT_STATUS_INVALID_NETWORK_RESPONSE;
+	}
+	if (windows_names) {
+		bad_name_p = strchr(name, '\\');
+		if (bad_name_p != NULL) {
+			/*
+			 * Windows names can't have '\\'.
+			 * Server is attacking us.
+			 */
+			return NT_STATUS_INVALID_NETWORK_RESPONSE;
+		}
+	}
+	return NT_STATUS_OK;
+}
+
+/****************************************************************************
+ Check if a returned directory name is safe. Disconnect if server is
+ sending bad names.
+****************************************************************************/
+
+NTSTATUS is_bad_finfo_name(const struct cli_state *cli,
+			const struct file_info *finfo)
+{
+	NTSTATUS status = NT_STATUS_OK;
+	bool windows_names = true;
+
+	if (cli->requested_posix_capabilities & CIFS_UNIX_POSIX_PATHNAMES_CAP) {
+		windows_names = false;
+	}
+	if (finfo->name != NULL) {
+		status = is_bad_name(windows_names, finfo->name);
+		if (!NT_STATUS_IS_OK(status)) {
+			DBG_ERR("bad finfo->name\n");
+			return status;
+		}
+	}
+	if (finfo->short_name != NULL) {
+		status = is_bad_name(windows_names, finfo->short_name);
+		if (!NT_STATUS_IS_OK(status)) {
+			DBG_ERR("bad finfo->short_name\n");
+			return status;
+		}
+	}
+	return NT_STATUS_OK;
+}
+
 /****************************************************************************
  Calculate a safe next_entry_offset.
 ****************************************************************************/
@@ -492,6 +552,13 @@ static NTSTATUS cli_list_old_recv(struct tevent_req *req, TALLOC_CTX *mem_ctx,
 			TALLOC_FREE(finfo);
 			return NT_STATUS_NO_MEMORY;
 		}
+
+		status = is_bad_finfo_name(state->cli, finfo);
+		if (!NT_STATUS_IS_OK(status)) {
+			smbXcli_conn_disconnect(state->cli->conn, status);
+			TALLOC_FREE(finfo);
+			return status;
+		}
 	}
 	*pfinfo = finfo;
 	return NT_STATUS_OK;
@@ -727,6 +794,14 @@ static void cli_list_trans_done(struct tevent_req *subreq)
 			ff_eos = true;
 			break;
 		}
+
+		status = is_bad_finfo_name(state->cli, finfo);
+		if (!NT_STATUS_IS_OK(status)) {
+			smbXcli_conn_disconnect(state->cli->conn, status);
+			tevent_req_nterror(req, status);
+			return;
+		}
+
 		if (!state->first && (state->mask[0] != '\0') &&
 		    strcsequal(finfo->name, state->mask)) {
 			DEBUG(1, ("Error: Looping in FIND_NEXT as name %s has "
diff --git a/source3/libsmb/proto.h b/source3/libsmb/proto.h
index 6a647da58c8..48855d7112c 100644
--- a/source3/libsmb/proto.h
+++ b/source3/libsmb/proto.h
@@ -760,6 +760,9 @@ NTSTATUS cli_posix_whoami(struct cli_state *cli,
 
 /* The following definitions come from libsmb/clilist.c  */
 
+NTSTATUS is_bad_finfo_name(const struct cli_state *cli,
+			const struct file_info *finfo);
+
 NTSTATUS cli_list_old(struct cli_state *cli,const char *Mask,uint16_t attribute,
 		      NTSTATUS (*fn)(const char *, struct file_info *,
 				 const char *, void *), void *state);
diff --git a/source4/dsdb/common/util.c b/source4/dsdb/common/util.c
index 817fce6e17f..bad2ee7a494 100644
--- a/source4/dsdb/common/util.c
+++ b/source4/dsdb/common/util.c
@@ -2041,21 +2041,36 @@ enum samr_ValidationStatus samdb_check_password(TALLOC_CTX *mem_ctx,
 						const uint32_t pwdProperties,
 						const uint32_t minPwdLength)
 {
-	const char *utf8_pw = (const char *)utf8_blob->data;
-	size_t utf8_len = strlen_m(utf8_pw);
 	char *password_script = NULL;
+	const char *utf8_pw = (const char *)utf8_blob->data;
+
+	/*
+	 * This looks strange because it is.
+	 *
+	 * The check for the number of characters in the password
+	 * should clearly not be against the byte length, or else a
+	 * single UTF8 character would count for more than one.
+	 *
+	 * We have chosen to use the number of 16-bit units that the
+	 * password encodes to as the measure of length.  This is not
+	 * the same as the number of codepoints, if a password
+	 * contains a character beyond the Basic Multilingual Plane
+	 * (above 65535) it will count for more than one "character".
+	 */
+
+	size_t password_characters_roughly = strlen_m(utf8_pw);
 
 	/* checks if the "minPwdLength" property is satisfied */
-	if (minPwdLength > utf8_len) {
+	if (minPwdLength > password_characters_roughly) {
 		return SAMR_VALIDATION_STATUS_PWD_TOO_SHORT;
 	}
 
-	/* checks the password complexity */
+	/* We might not be asked to check the password complexity */
 	if (!(pwdProperties & DOMAIN_PASSWORD_COMPLEX)) {
 		return SAMR_VALIDATION_STATUS_SUCCESS;
 	}
 
-	if (utf8_len == 0) {
+	if (password_characters_roughly == 0) {
 		return SAMR_VALIDATION_STATUS_NOT_COMPLEX_ENOUGH;
 	}
 
@@ -2063,6 +2078,7 @@ enum samr_ValidationStatus samdb_check_password(TALLOC_CTX *mem_ctx,
 	if (password_script != NULL && *password_script != '\0') {
 		int check_ret = 0;
 		int error = 0;
+		ssize_t nwritten = 0;
 		struct tevent_context *event_ctx = NULL;
 		struct tevent_req *req = NULL;
 		int cps_stdin = -1;
@@ -2125,7 +2141,9 @@ enum samr_ValidationStatus samdb_check_password(TALLOC_CTX *mem_ctx,
 
 		cps_stdin = samba_runcmd_export_stdin(req);
 
-		if (write(cps_stdin, utf8_pw, utf8_len) != utf8_len) {
+		nwritten = write(cps_stdin, utf8_blob->data,
+				 utf8_blob->length);
+		if (nwritten != utf8_blob->length) {
 			close(cps_stdin);
 			TALLOC_FREE(password_script);
 			TALLOC_FREE(event_ctx);


-- 
Samba Shared Repository



More information about the samba-cvs mailing list