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 <j...@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 <j...@samba.org>
Reviewed-by: Volker Lendecke <v...@samba.org>
Autobuild-User(master): Volker Lendecke <v...@samba.org>
Autobuild-Date(master): Mon Aug 14 15:55:43 UTC 2023 on atb-devel-224
commit 5bc50d2ea4444244721e72b4264311c7005d2f3c
Author: Jeremy Allison <j...@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 <v...@samba.org>
Signed-off-by: Jeremy Allison <j...@samba.org>
Reviewed-by: Volker Lendecke <v...@samba.org>
commit 9220c45cc191b34e293190f6a923ba463edd5db9
Author: Jeremy Allison <j...@samba.org>
Date: Fri Aug 11 10:42:41 2023 -0700
s3: smbd: Ensure srvstr_pull_req_talloc() always NULLs out *dest.
Robert Morris <r...@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 <j...@samba.org>
Reviewed-by: Volker Lendecke <v...@samba.org>
commit 963fd8aa9b76361ab9aeb63307773f2498b17879
Author: Jeremy Allison <j...@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 <j...@samba.org>
Reviewed-by: Volker Lendecke <v...@samba.org>
commit e7bf94b4e3a7f994aa6f0b859089c5add2ad380f
Author: Jeremy Allison <j...@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 <j...@samba.org>
Reviewed-by: Volker Lendecke <v...@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
