[SCM] Samba Shared Repository - branch master updated

Volker Lendecke vlendec at samba.org
Mon Aug 14 15:56:02 UTC 2023


The branch, master has been updated
       via  5379b8d557a s3: smbd: Ensure all callers to srvstr_pull_req_talloc() pass a zeroed-out dest pointer.
       via  5bc50d2ea44 s3: smbd: Uncorrupt the pointer we were using to prove a crash.
       via  9220c45cc19 s3: smbd: Ensure srvstr_pull_req_talloc() always NULLs out *dest.
       via  963fd8aa9b7 s3: torture: Add SMB1-TRUNCATED-SESSSETUP test.
       via  e7bf94b4e3a s3: smbd: Deliberately currupt an uninitialized pointer.
      from  c01c206d765 s4:kdc: Add get_claims_set_for_principal()

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


- Log -----------------------------------------------------------------
commit 5379b8d557a9a16b81eafb87b60b81debc4bfccb
Author: Jeremy Allison <jra at samba.org>
Date:   Fri Aug 11 10:52:31 2023 -0700

    s3: smbd: Ensure all callers to srvstr_pull_req_talloc() pass a zeroed-out dest pointer.
    
    Now we've fixed srvstr_pull_req_talloc() this isn't
    strictly needed, but ensuring pointers are initialized
    is best practice to avoid future bugs.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=15420
    
    Signed-off-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: Volker Lendecke <vl at samba.org>
    
    Autobuild-User(master): Volker Lendecke <vl at samba.org>
    Autobuild-Date(master): Mon Aug 14 15:55:43 UTC 2023 on atb-devel-224

commit 5bc50d2ea4444244721e72b4264311c7005d2f3c
Author: Jeremy Allison <jra at samba.org>
Date:   Fri Aug 11 10:47:28 2023 -0700

    s3: smbd: Uncorrupt the pointer we were using to prove a crash.
    
    Rather than restore to uninitialized, set to NULL as per
    modern coding practices.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=15420
    Reviewed-by: Volker Lendecke <vl at samba.org>
    
    Signed-off-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: Volker Lendecke <vl at samba.org>

commit 9220c45cc191b34e293190f6a923ba463edd5db9
Author: Jeremy Allison <jra at samba.org>
Date:   Fri Aug 11 10:42:41 2023 -0700

    s3: smbd: Ensure srvstr_pull_req_talloc() always NULLs out *dest.
    
    Robert Morris <rtm at lcs.mit.edu> noticed that in the case
    where srvstr_pull_req_talloc() is being called with
    buffer remaining == 0, we don't NULL out the destination
    pointed which is *always* done in the codepaths inside
    pull_string_talloc(). This prevents a crash in the caller.
    
    Remove knownfail.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=15420
    
    Signed-off-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: Volker Lendecke <vl at samba.org>

commit 963fd8aa9b76361ab9aeb63307773f2498b17879
Author: Jeremy Allison <jra at samba.org>
Date:   Fri Aug 11 10:39:36 2023 -0700

    s3: torture: Add SMB1-TRUNCATED-SESSSETUP test.
    
    Shows that we indirect through an uninitialized pointer and the client crashes
    it's own smbd.
    
    Add knownfail.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=15420
    
    Signed-off-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: Volker Lendecke <vl at samba.org>

commit e7bf94b4e3a7f994aa6f0b859089c5add2ad380f
Author: Jeremy Allison <jra at samba.org>
Date:   Fri Aug 11 10:38:23 2023 -0700

    s3: smbd: Deliberately currupt an uninitialized pointer.
    
    We will need this to show smbd crashing in the test code.
    This will be removed once we're passing the test.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=15420
    
    Signed-off-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: Volker Lendecke <vl at samba.org>

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

Summary of changes:
 source3/selftest/tests.py     |  11 +++
 source3/smbd/smb1_ipc.c       |   2 +-
 source3/smbd/smb1_message.c   |   2 +-
 source3/smbd/smb1_sesssetup.c |   4 +-
 source3/smbd/smb2_reply.c     |   1 +
 source3/torture/torture.c     | 181 ++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 197 insertions(+), 4 deletions(-)


Changeset truncated at 500 lines:

diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
index 461e09be87b..579ed87656d 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -219,6 +219,17 @@ plantestsuite("samba3.smbtorture_s3.hidenewfiles_showdirs",
                "",
                "-l $LOCAL_PATH"])
 
+plantestsuite("samba3.smbtorture_s3.smb1.SMB1-TRUNCATED-SESSSETUP",
+                "fileserver_smb1",
+                [os.path.join(samba3srcdir,
+                              "script/tests/test_smbtorture_s3.sh"),
+                'SMB1-TRUNCATED-SESSSETUP',
+                '//$SERVER_IP/tmp',
+                '$USERNAME',
+                '$PASSWORD',
+                smbtorture3,
+                "-mNT1"])
+
 #
 # MSDFS attribute tests.
 #
diff --git a/source3/smbd/smb1_ipc.c b/source3/smbd/smb1_ipc.c
index 3f9958fece0..716b67b40ea 100644
--- a/source3/smbd/smb1_ipc.c
+++ b/source3/smbd/smb1_ipc.c
@@ -695,7 +695,7 @@ void reply_trans(struct smb_request *req)
 		return;
 	}
 
-	if ((state = talloc(conn, struct trans_state)) == NULL) {
+	if ((state = talloc_zero(conn, struct trans_state)) == NULL) {
 		DEBUG(0, ("talloc failed\n"));
 		reply_nterror(req, NT_STATUS_NO_MEMORY);
 		END_PROFILE(SMBtrans);
diff --git a/source3/smbd/smb1_message.c b/source3/smbd/smb1_message.c
index 928be77f854..ca7201e2e7f 100644
--- a/source3/smbd/smb1_message.c
+++ b/source3/smbd/smb1_message.c
@@ -159,7 +159,7 @@ void reply_sends(struct smb_request *req)
 		return;
 	}
 
-	state = talloc(talloc_tos(), struct msg_state);
+	state = talloc_zero(talloc_tos(), struct msg_state);
 
 	p = req->buf + 1;
 	p += srvstr_pull_req_talloc(
diff --git a/source3/smbd/smb1_sesssetup.c b/source3/smbd/smb1_sesssetup.c
index fe4519aef20..6c668fffa7b 100644
--- a/source3/smbd/smb1_sesssetup.c
+++ b/source3/smbd/smb1_sesssetup.c
@@ -86,7 +86,7 @@ static void reply_sesssetup_and_X_spnego(struct smb_request *req)
 	DATA_BLOB in_blob;
 	DATA_BLOB out_blob = data_blob_null;
 	size_t bufrem;
-	char *tmp;
+	char *tmp = NULL;
 	const char *native_os;
 	const char *native_lanman;
 	const char *primary_domain;
@@ -581,7 +581,7 @@ void reply_sesssetup_and_X(struct smb_request *req)
 	struct reply_sesssetup_and_X_state *state = NULL;
 	uint64_t sess_vuid;
 	uint16_t smb_bufsize;
-	char *tmp;
+	char *tmp = NULL;
 	fstring sub_user; /* Sanitised username for substitution */
 	const char *native_os;
 	const char *native_lanman;
diff --git a/source3/smbd/smb2_reply.c b/source3/smbd/smb2_reply.c
index 66b735e0b75..dfcd05d2cae 100644
--- a/source3/smbd/smb2_reply.c
+++ b/source3/smbd/smb2_reply.c
@@ -533,6 +533,7 @@ size_t srvstr_pull_req_talloc(TALLOC_CTX *ctx, struct smb_request *req,
 	ssize_t bufrem = smbreq_bufrem(req, src);
 
 	if (bufrem == 0) {
+		*dest = NULL;
 		return 0;
 	}
 
diff --git a/source3/torture/torture.c b/source3/torture/torture.c
index ab992665323..59ef401197d 100644
--- a/source3/torture/torture.c
+++ b/source3/torture/torture.c
@@ -14645,6 +14645,182 @@ static bool run_local_canonicalize_path(int dummy)
 	}
 	return true;
 }
+struct session_setup_nt1_truncated_state {
+	uint16_t vwv[13];
+	uint8_t bytes[20];
+};
+
+static void smb1_session_setup_nt1_truncated_done(struct tevent_req *subreq);
+
+static struct tevent_req *smb1_session_setup_nt1_truncated_send(
+		TALLOC_CTX *mem_ctx,
+		struct tevent_context *ev,
+		struct smbXcli_conn *conn)
+{
+	uint16_t *vwv = NULL;
+	uint8_t *bytes = NULL;
+	const char *pass = "12345678";
+	const char *uname = "z";
+	struct session_setup_nt1_truncated_state *state = NULL;
+	struct tevent_req *req = NULL;
+	struct tevent_req *subreq = NULL;
+
+	req = tevent_req_create(mem_ctx,
+				&state,
+				struct session_setup_nt1_truncated_state);
+	if (req == NULL) {
+		return NULL;
+	}
+	vwv = &state->vwv[0];
+	bytes = &state->bytes[0];
+
+	SCVAL(vwv+0,  0, 0xff);
+	SCVAL(vwv+0,  1, 0);
+	SSVAL(vwv+1,  0, 0);
+	SSVAL(vwv+2,  0, 8192);
+	SSVAL(vwv+3,  0, 2);
+	SSVAL(vwv+4,  0, 1);
+	SIVAL(vwv+5,  0, 0);
+	SSVAL(vwv+7,  0, strlen(pass)); /* OEMPasswordLen */
+	SSVAL(vwv+8,  0, 0); /* UnicodePasswordLen */
+	SSVAL(vwv+9,  0, 0); /* reserved */
+	SSVAL(vwv+10, 0, 0); /* reserved */
+	SIVAL(vwv+11, 0, CAP_STATUS32);
+
+	memcpy(bytes, pass, strlen(pass));
+	bytes += strlen(pass);
+	memcpy(bytes, uname, strlen(uname)+1);
+
+	subreq = smb1cli_req_send(state, ev, conn,
+				  SMBsesssetupX,
+				  0, /*  additional_flags */
+				  0, /*  clear_flags */
+				  0, /*  additional_flags2 */
+				  0, /*  clear_flags2 */
+				  10000, /* timeout_msec */
+				  getpid(),
+				  NULL, /* tcon */
+				  NULL, /* session */
+				  13, /* wct */
+				  state->vwv,
+				  strlen(pass), /* Truncate length at password. */
+				  state->bytes);
+	if (tevent_req_nomem(subreq, req)) {
+		return tevent_req_post(req, ev);
+	}
+	tevent_req_set_callback(subreq,
+				smb1_session_setup_nt1_truncated_done,
+				req);
+	return req;
+}
+
+static void smb1_session_setup_nt1_truncated_done(struct tevent_req *subreq)
+{
+	struct tevent_req *req =
+		tevent_req_callback_data(subreq,
+		struct tevent_req);
+	struct session_setup_nt1_truncated_state *state =
+		tevent_req_data(req,
+		struct session_setup_nt1_truncated_state);
+	NTSTATUS status;
+	struct smb1cli_req_expected_response expected[] = {
+	{
+		.status = NT_STATUS_OK,
+		.wct    = 3,
+	},
+	};
+
+	status = smb1cli_req_recv(subreq, state,
+				  NULL,
+				  NULL,
+				  NULL,
+				  NULL,
+				  NULL, /* pvwv_offset */
+				  NULL,
+				  NULL,
+				  NULL, /* pbytes_offset */
+				  NULL,
+				  expected, ARRAY_SIZE(expected));
+	TALLOC_FREE(subreq);
+	if (tevent_req_nterror(req, status)) {
+		return;
+	}
+	tevent_req_done(req);
+}
+
+static NTSTATUS smb1_session_setup_nt1_truncated_recv(struct tevent_req *req)
+{
+	return tevent_req_simple_recv_ntstatus(req);
+}
+
+static bool run_smb1_truncated_sesssetup(int dummy)
+{
+	struct tevent_context *ev;
+	struct tevent_req *req;
+	struct smbXcli_conn *conn;
+	struct sockaddr_storage ss;
+	NTSTATUS status;
+	int fd;
+	bool ok;
+
+	printf("Starting send truncated SMB1 sesssetup.\n");
+
+	ok = resolve_name(host, &ss, 0x20, true);
+	if (!ok) {
+		d_fprintf(stderr, "Could not resolve name %s\n", host);
+		return false;
+	}
+
+	status = open_socket_out(&ss, 445, 10000, &fd);
+	if (!NT_STATUS_IS_OK(status)) {
+		d_fprintf(stderr, "open_socket_out failed: %s\n",
+			  nt_errstr(status));
+		return false;
+	}
+
+	conn = smbXcli_conn_create(talloc_tos(), fd, host, SMB_SIGNING_OFF, 0,
+				   NULL, 0, NULL);
+	if (conn == NULL) {
+		d_fprintf(stderr, "smbXcli_conn_create failed\n");
+		return false;
+	}
+
+	status = smbXcli_negprot(conn, 0, PROTOCOL_NT1, PROTOCOL_NT1);
+	if (!NT_STATUS_IS_OK(status)) {
+		d_fprintf(stderr, "smbXcli_negprot failed!\n");
+		return false;
+	}
+
+	ev = samba_tevent_context_init(talloc_tos());
+	if (ev == NULL) {
+		d_fprintf(stderr, "samba_tevent_context_init failed\n");
+		return false;
+	}
+
+	req = smb1_session_setup_nt1_truncated_send(ev, ev, conn);
+	if (req == NULL) {
+		d_fprintf(stderr, "smb1_session_setup_nt1_truncated_send failed\n");
+		return false;
+	}
+
+	ok = tevent_req_poll_ntstatus(req, ev, &status);
+	if (!ok) {
+		d_fprintf(stderr, "tevent_req_poll failed with status %s\n",
+			nt_errstr(status));
+		return false;
+	}
+
+	status = smb1_session_setup_nt1_truncated_recv(req);
+	if (!NT_STATUS_IS_OK(status)) {
+		d_fprintf(stderr, "smb1_session_setup_nt1_truncated_recv returned "
+			  "%s, expected NT_STATUS_OK\n",
+			  nt_errstr(status));
+		return false;
+	}
+
+	TALLOC_FREE(conn);
+	return true;
+}
 
 static bool run_ign_bad_negprot(int dummy)
 {
@@ -14721,6 +14897,7 @@ static bool run_ign_bad_negprot(int dummy)
 	return true;
 }
 
+
 static double create_procs(bool (*fn)(int), bool *result)
 {
 	int i, status;
@@ -15373,6 +15550,10 @@ static struct {
 		.name  = "SMB2-DFS-FILENAME-LEADING-BACKSLASH",
 		.fn    = run_smb2_dfs_filename_leading_backslash,
 	},
+	{
+		.name  = "SMB1-TRUNCATED-SESSSETUP",
+		.fn    = run_smb1_truncated_sesssetup,
+	},
 	{
 		.name  = "SMB1-DFS-PATHS",
 		.fn    = run_smb1_dfs_paths,


-- 
Samba Shared Repository



More information about the samba-cvs mailing list