[SCM] Samba Shared Repository - branch master updated

Karolin Seeger kseeger at samba.org
Thu Oct 29 11:48:03 UTC 2020


The branch, master has been updated
       via  3076566d656 s3: smbd: Ensure change notifies can't get set unless the directory handle is open for SEC_DIR_LIST.
       via  6e143d9c7a6 s4: torture: Add smb2.notify.handle-permissions test.
       via  40f23c24598 CVE-2020-14323 torture4: Add a simple test for invalid lookup_sids winbind call
       via  a380f19d570 CVE-2020-14323 winbind: Fix invalid lookupsids DoS
      from  cc4901123da s3:rpcclient fix NULL - deref caused by misuse of chgpasswd3

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


- Log -----------------------------------------------------------------
commit 3076566d6565113edce9c330b196c93f8cc5aa11
Author: Jeremy Allison <jra at samba.org>
Date:   Tue Jul 7 18:25:23 2020 -0700

    s3: smbd: Ensure change notifies can't get set unless the directory handle is open for SEC_DIR_LIST.
    
    Remove knownfail entry.
    
    CVE-2020-14318
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14434
    
    Signed-off-by: Jeremy Allison <jra at samba.org>
    
    Autobuild-User(master): Karolin Seeger <kseeger at samba.org>
    Autobuild-Date(master): Thu Oct 29 11:47:35 UTC 2020 on sn-devel-184

commit 6e143d9c7a6a68fa8c99708ec01fbfd389327426
Author: Jeremy Allison <jra at samba.org>
Date:   Fri Jul 10 15:09:33 2020 -0700

    s4: torture: Add smb2.notify.handle-permissions test.
    
    Add knownfail entry.
    
    CVE-2020-14318
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14434
    
    Signed-off-by: Jeremy Allison <jra at samba.org>

commit 40f23c24598aee5456e41c813a37ccdc759ed000
Author: Volker Lendecke <vl at samba.org>
Date:   Thu Jul 9 21:48:57 2020 +0200

    CVE-2020-14323 torture4: Add a simple test for invalid lookup_sids winbind call
    
    We can't add this test before the fix, add it to knownfail and have the fix
    remove the knownfail entry again. As this crashes winbind, many tests after
    this one will fail.
    
    Reported by Bas Alberts of the GitHub Security Lab Team as GHSL-2020-134
    
    Bug: https://bugzilla.samba.org/show_bug.cgi?id=14436
    Signed-off-by: Volker Lendecke <vl at samba.org>

commit a380f19d570003c0134e5a9618fbeee524ca332a
Author: Volker Lendecke <vl at samba.org>
Date:   Thu Jul 9 21:49:25 2020 +0200

    CVE-2020-14323 winbind: Fix invalid lookupsids DoS
    
    A lookupsids request without extra_data will lead to "state->domain==NULL",
    which makes winbindd_lookupsids_recv trying to dereference it.
    
    Reported by Bas Alberts of the GitHub Security Lab Team as GHSL-2020-134
    
    Bug: https://bugzilla.samba.org/show_bug.cgi?id=14436
    Signed-off-by: Volker Lendecke <vl at samba.org>

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

Summary of changes:
 source3/smbd/notify.c                  |  8 ++++
 source3/winbindd/winbindd_lookupsids.c |  2 +-
 source4/torture/smb2/notify.c          | 80 ++++++++++++++++++++++++++++++++++
 source4/torture/winbind/struct_based.c | 27 ++++++++++++
 4 files changed, 116 insertions(+), 1 deletion(-)


Changeset truncated at 500 lines:

diff --git a/source3/smbd/notify.c b/source3/smbd/notify.c
index eb6317b7e8a..5f18b5cf794 100644
--- a/source3/smbd/notify.c
+++ b/source3/smbd/notify.c
@@ -289,6 +289,14 @@ NTSTATUS change_notify_create(struct files_struct *fsp,
 	char fullpath[len+1];
 	NTSTATUS status = NT_STATUS_NOT_IMPLEMENTED;
 
+	/*
+	 * Setting a changenotify needs READ/LIST access
+	 * on the directory handle.
+	 */
+	if (!(fsp->access_mask & SEC_DIR_LIST)) {
+		return NT_STATUS_ACCESS_DENIED;
+	}
+
 	if (fsp->notify != NULL) {
 		DEBUG(1, ("change_notify_create: fsp->notify != NULL, "
 			  "fname = %s\n", fsp->fsp_name->base_name));
diff --git a/source3/winbindd/winbindd_lookupsids.c b/source3/winbindd/winbindd_lookupsids.c
index d28b5fa9f01..a289fd86f0f 100644
--- a/source3/winbindd/winbindd_lookupsids.c
+++ b/source3/winbindd/winbindd_lookupsids.c
@@ -47,7 +47,7 @@ struct tevent_req *winbindd_lookupsids_send(TALLOC_CTX *mem_ctx,
 	DEBUG(3, ("lookupsids\n"));
 
 	if (request->extra_len == 0) {
-		tevent_req_done(req);
+		tevent_req_nterror(req, NT_STATUS_INVALID_PARAMETER);
 		return tevent_req_post(req, ev);
 	}
 	if (request->extra_data.data[request->extra_len-1] != '\0') {
diff --git a/source4/torture/smb2/notify.c b/source4/torture/smb2/notify.c
index b65c116b75e..6081d394c6e 100644
--- a/source4/torture/smb2/notify.c
+++ b/source4/torture/smb2/notify.c
@@ -2649,6 +2649,83 @@ done:
 	return ok;
 }
 
+/*
+  Test asking for a change notify on a handle without permissions.
+*/
+
+#define BASEDIR_HPERM BASEDIR "_HPERM"
+
+static bool torture_smb2_notify_handle_permissions(
+		struct torture_context *torture,
+		struct smb2_tree *tree)
+{
+	bool ret = true;
+	NTSTATUS status;
+	union smb_notify notify;
+	union smb_open io;
+	struct smb2_handle h1 = {{0}};
+	struct smb2_request *req;
+
+	smb2_deltree(tree, BASEDIR_HPERM);
+	smb2_util_rmdir(tree, BASEDIR_HPERM);
+
+	torture_comment(torture,
+		"TESTING CHANGE NOTIFY "
+		"ON A HANDLE WITHOUT PERMISSIONS\n");
+
+	/*
+	  get a handle on the directory
+	*/
+	ZERO_STRUCT(io.smb2);
+	io.generic.level = RAW_OPEN_SMB2;
+	io.smb2.in.create_flags = 0;
+	io.smb2.in.desired_access = SEC_FILE_READ_ATTRIBUTE;
+	io.smb2.in.create_options = NTCREATEX_OPTIONS_DIRECTORY;
+	io.smb2.in.file_attributes = FILE_ATTRIBUTE_NORMAL;
+	io.smb2.in.share_access = NTCREATEX_SHARE_ACCESS_READ |
+				NTCREATEX_SHARE_ACCESS_WRITE;
+	io.smb2.in.alloc_size = 0;
+	io.smb2.in.create_disposition = NTCREATEX_DISP_CREATE;
+	io.smb2.in.impersonation_level = SMB2_IMPERSONATION_ANONYMOUS;
+	io.smb2.in.security_flags = 0;
+	io.smb2.in.fname = BASEDIR_HPERM;
+
+	status = smb2_create(tree, torture, &io.smb2);
+	CHECK_STATUS(status, NT_STATUS_OK);
+	h1 = io.smb2.out.file.handle;
+
+	/* ask for a change notify,
+	   on file or directory name changes */
+	ZERO_STRUCT(notify.smb2);
+	notify.smb2.level = RAW_NOTIFY_SMB2;
+	notify.smb2.in.buffer_size = 1000;
+	notify.smb2.in.completion_filter = FILE_NOTIFY_CHANGE_NAME;
+	notify.smb2.in.file.handle = h1;
+	notify.smb2.in.recursive = true;
+
+	req = smb2_notify_send(tree, &notify.smb2);
+	torture_assert_goto(torture,
+			req != NULL,
+			ret,
+			done,
+			"smb2_notify_send failed\n");
+
+	/*
+	 * Cancel it, we don't really want to wait.
+	 */
+	smb2_cancel(req);
+	status = smb2_notify_recv(req, torture, &notify.smb2);
+	/* Handle h1 doesn't have permissions for ChangeNotify. */
+	CHECK_STATUS(status, NT_STATUS_ACCESS_DENIED);
+
+done:
+	if (!smb2_util_handle_empty(h1)) {
+		smb2_util_close(tree, h1);
+	}
+	smb2_deltree(tree, BASEDIR_HPERM);
+	return ret;
+}
+
 /*
    basic testing of SMB2 change notify
 */
@@ -2682,6 +2759,9 @@ struct torture_suite *torture_smb2_notify_init(TALLOC_CTX *ctx)
 				     torture_smb2_notify_rmdir3);
 	torture_suite_add_2smb2_test(suite, "rmdir4",
 				     torture_smb2_notify_rmdir4);
+	torture_suite_add_1smb2_test(suite,
+				    "handle-permissions",
+				    torture_smb2_notify_handle_permissions);
 
 	suite->description = talloc_strdup(suite, "SMB2-NOTIFY tests");
 
diff --git a/source4/torture/winbind/struct_based.c b/source4/torture/winbind/struct_based.c
index 90c70c61069..724b14cbbb7 100644
--- a/source4/torture/winbind/struct_based.c
+++ b/source4/torture/winbind/struct_based.c
@@ -1111,6 +1111,29 @@ static bool torture_winbind_struct_lookup_name_sid(struct torture_context *tortu
 	return true;
 }
 
+static bool torture_winbind_struct_lookup_sids_invalid(
+	struct torture_context *torture)
+{
+	struct winbindd_request req = {0};
+	struct winbindd_response rep = {0};
+	bool strict = torture_setting_bool(torture, "strict mode", false);
+	bool ok;
+
+	torture_comment(torture,
+			"Running WINBINDD_LOOKUP_SIDS (struct based)\n");
+
+	ok = true;
+	DO_STRUCT_REQ_REP_EXT(WINBINDD_LOOKUPSIDS, &req, &rep,
+			      NSS_STATUS_NOTFOUND,
+			      strict,
+			      ok=false,
+			      talloc_asprintf(
+				      torture,
+				      "invalid lookupsids succeeded"));
+
+	return ok;
+}
+
 struct torture_suite *torture_winbind_struct_init(TALLOC_CTX *ctx)
 {
 	struct torture_suite *suite = torture_suite_create(ctx, "struct");
@@ -1133,6 +1156,10 @@ struct torture_suite *torture_winbind_struct_init(TALLOC_CTX *ctx)
 	torture_suite_add_simple_test(suite, "getpwent", torture_winbind_struct_getpwent);
 	torture_suite_add_simple_test(suite, "endpwent", torture_winbind_struct_endpwent);
 	torture_suite_add_simple_test(suite, "lookup_name_sid", torture_winbind_struct_lookup_name_sid);
+	torture_suite_add_simple_test(
+		suite,
+		"lookup_sids_invalid",
+		torture_winbind_struct_lookup_sids_invalid);
 
 	suite->description = talloc_strdup(suite, "WINBIND - struct based protocol tests");
 


-- 
Samba Shared Repository



More information about the samba-cvs mailing list