The Samba-Bugzilla – Attachment 18056 Details for
Bug 15432
TREE_CONNECT without SETUP causes smbd to use uninitialized pointer
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
git-am fix for 4.17.next
bug-15432-4.17.patch (text/plain), 6.97 KB, created by
Jeremy Allison
on 2023-08-15 17:35:14 UTC
(
hide
)
Description:
git-am fix for 4.17.next
Filename:
MIME Type:
Creator:
Jeremy Allison
Created:
2023-08-15 17:35:14 UTC
Size:
6.97 KB
patch
obsolete
>From 74d4b1cd53e666bb037df87612d412f77f92e630 Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Fri, 11 Aug 2023 17:14:38 -0700 >Subject: [PATCH 1/3] 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 <jra@samba.org> >Reviewed-by: Noel Power <npower@samba.org> >(Back-ported from commit f02f74e931f5821c7b7c1be2b8f0fb60c9a69b19) >--- > source3/smbd/smb2_process.c | 1 + > 1 file changed, 1 insertion(+) > >diff --git a/source3/smbd/smb2_process.c b/source3/smbd/smb2_process.c >index ad4386e08a4..e003cbaf481 100644 >--- a/source3/smbd/smb2_process.c >+++ b/source3/smbd/smb2_process.c >@@ -793,6 +793,7 @@ bool init_smb1_request(struct smb_request *req, > req->smb2req = NULL; > req->chain = NULL; > req->posix_pathnames = lp_posix_pathnames(); >+ req->session = (void *)0xDEADBEEF; > smb_init_perfcount_data(&req->pcd); > > /* Ensure we have at least wct words and 2 bytes of bcc. */ >-- >2.34.1 > > >From 7b0bfa931721bfff9c0ab80358cc442d1d97da23 Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Fri, 11 Aug 2023 17:18:26 -0700 >Subject: [PATCH 2/3] s3: torture: Add SMB1-NEGOTIATE-TCON that shows the SMB1 > server crashes on the uninitialized req->session. > >Found by Robert Morris <rtm@lcs.mit.edu>. > >Adds knownfail. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15432 > >Signed-off-by: Jeremy Allison <jra@samba.org> >Reviewed-by: Noel Power <nopower@samba.org> >(Back-ported from commit c32df3bb31ce6275cfb91107e34e2d6b3c2fba1b) >--- > selftest/knownfail.d/smb1_negprot_tcon | 1 + > source3/selftest/tests.py | 11 +++++++ > source3/torture/torture.c | 40 ++++++++++++++++++++++++++ > 3 files changed, 52 insertions(+) > create mode 100644 selftest/knownfail.d/smb1_negprot_tcon > >diff --git a/selftest/knownfail.d/smb1_negprot_tcon b/selftest/knownfail.d/smb1_negprot_tcon >new file mode 100644 >index 00000000000..4f620948c31 >--- /dev/null >+++ b/selftest/knownfail.d/smb1_negprot_tcon >@@ -0,0 +1 @@ >+^samba3.smbtorture_s3.smb1.SMB1-NEGOTIATE-TCON.smbtorture\(fileserver_smb1\) >diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py >index 62f4662c9a3..831fdd6db2e 100755 >--- a/source3/selftest/tests.py >+++ b/source3/selftest/tests.py >@@ -225,6 +225,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/torture/torture.c b/source3/torture/torture.c >index 689fe494c14..4b22958c838 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 = "PIDHIGH", > .fn = run_pidhigh, >-- >2.34.1 > > >From 184afd7ca184c39961f03bac3042a51a8d7e7d38 Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Fri, 11 Aug 2023 17:28:53 -0700 >Subject: [PATCH 3/3] 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 <jra@samba.org> >Reviewed-by: Noel Power <npower@samba.org> > >Autobuild-User(master): Noel Power <npower@samba.org> >Autobuild-Date(master): Tue Aug 15 12:06:36 UTC 2023 on atb-devel-224 > >(Back-ported from commit 4145bfb1b5a3639caf26a310d612aec29fc00117) >--- > selftest/knownfail.d/smb1_negprot_tcon | 1 - > source3/smbd/smb2_process.c | 8 ++------ > 2 files changed, 2 insertions(+), 7 deletions(-) > delete mode 100644 selftest/knownfail.d/smb1_negprot_tcon > >diff --git a/selftest/knownfail.d/smb1_negprot_tcon b/selftest/knownfail.d/smb1_negprot_tcon >deleted file mode 100644 >index 4f620948c31..00000000000 >--- a/selftest/knownfail.d/smb1_negprot_tcon >+++ /dev/null >@@ -1 +0,0 @@ >-^samba3.smbtorture_s3.smb1.SMB1-NEGOTIATE-TCON.smbtorture\(fileserver_smb1\) >diff --git a/source3/smbd/smb2_process.c b/source3/smbd/smb2_process.c >index e003cbaf481..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,18 +784,13 @@ 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(); >- req->session = (void *)0xDEADBEEF; > smb_init_perfcount_data(&req->pcd); > > /* Ensure we have at least wct words and 2 bytes of bcc. */ >@@ -813,7 +810,6 @@ bool init_smb1_request(struct smb_request *req, > return false; > } > >- req->outbuf = NULL; > return true; > } > >-- >2.34.1 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Raw
Flags:
npower
:
review+
Actions:
View
Attachments on
bug 15432
:
17997
|
18045
|
18054
|
18055
| 18056