The branch, v4-18-test has been updated via 0a044e409de s3: smbd: Ensure init_smb1_request() zeros out what the incoming pointer points to. via 0605946d20f s3: torture: Add SMB1-NEGOTIATE-TCON that shows the SMB1 server crashes on the uninitialized req->session. via d2c16aada79 s3: smbd: init_smb1_request() isn't being passed zero'ed memory from any codepath. from fec8cda70bc VERSION: Bump version up to Samba 4.18.7...
https://git.samba.org/?p=samba.git;a=shortlog;h=v4-18-test - Log ----------------------------------------------------------------- commit 0a044e409dede6a51c32d269e82c17f6b8f5abf0 Author: Jeremy Allison <j...@samba.org> Date: Fri Aug 11 17:28:53 2023 -0700 s3: smbd: Ensure init_smb1_request() zeros out what the incoming pointer points to. Remove the now unneeded req->xxx = NULL assignments (and the deliberately bogus req->session = (void *)0xDEADBEEF one used to demonstrate the bug). Remove knownfail. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15432 Signed-off-by: Jeremy Allison <j...@samba.org> Reviewed-by: Noel Power <npo...@samba.org> Autobuild-User(master): Noel Power <npo...@samba.org> Autobuild-Date(master): Tue Aug 15 12:06:36 UTC 2023 on atb-devel-224 (Back-ported from commit 4145bfb1b5a3639caf26a310d612aec29fc00117) Autobuild-User(v4-18-test): Jule Anger <jan...@samba.org> Autobuild-Date(v4-18-test): Fri Aug 18 14:07:39 UTC 2023 on atb-devel-224 commit 0605946d20f582cb66a807baf3defa41b97f4857 Author: Jeremy Allison <j...@samba.org> Date: Fri Aug 11 17:18:26 2023 -0700 s3: torture: Add SMB1-NEGOTIATE-TCON that shows the SMB1 server crashes on the uninitialized req->session. Found by Robert Morris <r...@lcs.mit.edu>. Adds knownfail. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15432 Signed-off-by: Jeremy Allison <j...@samba.org> Reviewed-by: Noel Power <nopo...@samba.org> (Back-ported from commit c32df3bb31ce6275cfb91107e34e2d6b3c2fba1b) commit d2c16aada79fa13dcc2e60b05586388f819d93d6 Author: Jeremy Allison <j...@samba.org> Date: Fri Aug 11 17:14:38 2023 -0700 s3: smbd: init_smb1_request() isn't being passed zero'ed memory from any codepath. If a client does a SMB1 NEGPROT followed by SMB1 TCON then req->session is left uninitialized. Show this causes a crash by deliberately initializing req->session to an invalid pointer. This will be removed once the test shows the crash, and the fix is added to cause init_smb1_request() to zero the memory passed in. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15432 Signed-off-by: Jeremy Allison <j...@samba.org> Reviewed-by: Noel Power <npo...@samba.org> (Back-ported from commit f02f74e931f5821c7b7c1be2b8f0fb60c9a69b19) ----------------------------------------------------------------------- Summary of changes: source3/selftest/tests.py | 11 +++++++++++ source3/smbd/smb2_process.c | 7 ++----- source3/torture/torture.c | 40 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 53 insertions(+), 5 deletions(-) Changeset truncated at 500 lines: diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py index 178f550b5b7..809a2be631b 100755 --- a/source3/selftest/tests.py +++ b/source3/selftest/tests.py @@ -239,6 +239,17 @@ plantestsuite("samba3.smbtorture_s3.smb1.SMB1-NEGOTIATE-EXIT", smbtorture3, "-mNT1"]) +plantestsuite("samba3.smbtorture_s3.smb1.SMB1-NEGOTIATE-TCON", + "fileserver_smb1", + [os.path.join(samba3srcdir, + "script/tests/test_smbtorture_s3.sh"), + 'SMB1-NEGOTIATE-TCON', + '//$SERVER_IP/tmp', + '$USERNAME', + '$PASSWORD', + smbtorture3, + "-mNT1"]) + # # MSDFS attribute tests. # diff --git a/source3/smbd/smb2_process.c b/source3/smbd/smb2_process.c index ad4386e08a4..11f556c88ac 100644 --- a/source3/smbd/smb2_process.c +++ b/source3/smbd/smb2_process.c @@ -764,6 +764,8 @@ bool init_smb1_request(struct smb_request *req, return false; } + *req = (struct smb_request) { .cmd = 0}; + req->request_time = timeval_current(); now = timeval_to_nttime(&req->request_time); @@ -782,16 +784,12 @@ bool init_smb1_request(struct smb_request *req, req->encrypted = encrypted; req->sconn = sconn; req->xconn = xconn; - req->conn = NULL; if (xconn != NULL) { status = smb1srv_tcon_lookup(xconn, req->tid, now, &tcon); if (NT_STATUS_IS_OK(status)) { req->conn = tcon->compat; } } - req->chain_fsp = NULL; - req->smb2req = NULL; - req->chain = NULL; req->posix_pathnames = lp_posix_pathnames(); smb_init_perfcount_data(&req->pcd); @@ -812,7 +810,6 @@ bool init_smb1_request(struct smb_request *req, return false; } - req->outbuf = NULL; return true; } diff --git a/source3/torture/torture.c b/source3/torture/torture.c index 63fe4ac6f7f..87ac0678526 100644 --- a/source3/torture/torture.c +++ b/source3/torture/torture.c @@ -14999,6 +14999,42 @@ static bool run_smb1_negotiate_exit(int dummy) return true; } +static bool run_smb1_negotiate_tcon(int dummy) +{ + struct cli_state *cli = NULL; + uint16_t cnum = 0; + uint16_t max_xmit = 0; + NTSTATUS status; + + printf("Starting send SMB1 negotiate+tcon.\n"); + cli = open_nbt_connection(); + if (cli == NULL) { + d_fprintf(stderr, "open_nbt_connection failed!\n"); + return false; + } + smbXcli_conn_set_sockopt(cli->conn, sockops); + + status = smbXcli_negprot(cli->conn, 0, PROTOCOL_NT1, PROTOCOL_NT1); + if (!NT_STATUS_IS_OK(status)) { + d_fprintf(stderr, "smbXcli_negprot failed %s!\n", + nt_errstr(status)); + return false; + } + status = cli_raw_tcon(cli, + share, + "", + "?????", + &max_xmit, + &cnum); + if (!NT_STATUS_EQUAL(status, NT_STATUS_ACCESS_DENIED)) { + d_fprintf(stderr, "cli_raw_tcon failed - got %s " + "(should get NT_STATUS_ACCESS_DENIED)!\n", + nt_errstr(status)); + return false; + } + return true; +} + static bool run_ign_bad_negprot(int dummy) { struct tevent_context *ev; @@ -15735,6 +15771,10 @@ static struct { .name = "SMB1-NEGOTIATE-EXIT", .fn = run_smb1_negotiate_exit, }, + { + .name = "SMB1-NEGOTIATE-TCON", + .fn = run_smb1_negotiate_tcon, + }, { .name = "SMB1-DFS-PATHS", .fn = run_smb1_dfs_paths, -- Samba Shared Repository