From 7428b86fc82e084484331ad1f8c222fa168b3e40 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 26 Jul 2023 16:37:11 -0700 Subject: [PATCH 1/2] s3: torture: Add test to show an SMB1 DFS path of "\\x//\\/" crashes smbd. Adds knownfail. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15419 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme (cherry picked from commit 2aa9ffa2f0fc79599efbfe0c37aac4ef5160f712) --- selftest/knownfail.d/dfs_badpath | 1 + source3/selftest/tests.py | 14 ++++++++ source3/torture/proto.h | 1 + source3/torture/test_smb1_dfs.c | 56 ++++++++++++++++++++++++++++++++ source3/torture/torture.c | 4 +++ 5 files changed, 76 insertions(+) create mode 100644 selftest/knownfail.d/dfs_badpath diff --git a/selftest/knownfail.d/dfs_badpath b/selftest/knownfail.d/dfs_badpath new file mode 100644 index 00000000000..9fd16e99075 --- /dev/null +++ b/selftest/knownfail.d/dfs_badpath @@ -0,0 +1 @@ +^samba3.smbtorture_s3.smb1.SMB1-DFS-BADPATH.smbtorture\(fileserver_smb1\) diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py index afe408e0f64..76c518f3ae1 100755 --- a/source3/selftest/tests.py +++ b/source3/selftest/tests.py @@ -355,6 +355,20 @@ plantestsuite("samba3.smbtorture_s3.smb1.SMB1-DFS-OPERATIONS", '$PASSWORD', smbtorture3, "-mNT1"]) +# +# SMB1-DFS-BADPATH needs to run against a special share msdfs-pathname-share +# BUG: https://bugzilla.samba.org/show_bug.cgi?id=15419 +# +plantestsuite("samba3.smbtorture_s3.smb1.SMB1-DFS-BADPATH", + "fileserver_smb1", + [os.path.join(samba3srcdir, + "script/tests/test_smbtorture_s3.sh"), + 'SMB1-DFS-BADPATH', + '//$SERVER_IP/msdfs-pathname-share', + '$USERNAME', + '$PASSWORD', + smbtorture3, + "-mNT1"]) # # SMB2-STREAM-ACL needs to run against a special share - vfs_wo_fruit diff --git a/source3/torture/proto.h b/source3/torture/proto.h index 5e6d914c3da..5bfe4cc8e02 100644 --- a/source3/torture/proto.h +++ b/source3/torture/proto.h @@ -127,6 +127,7 @@ bool run_smb2_dfs_filename_leading_backslash(int dummy); bool run_smb1_dfs_paths(int dummy); bool run_smb1_dfs_search_paths(int dummy); bool run_smb1_dfs_operations(int dummy); +bool run_smb1_dfs_check_badpath(int dummy); bool run_list_dir_async_test(int dummy); bool run_delete_on_close_non_empty(int dummy); bool run_delete_on_close_nonwrite_delete_yes_test(int dummy); diff --git a/source3/torture/test_smb1_dfs.c b/source3/torture/test_smb1_dfs.c index 5dfa7b41b6e..d30438d0824 100644 --- a/source3/torture/test_smb1_dfs.c +++ b/source3/torture/test_smb1_dfs.c @@ -3814,6 +3814,26 @@ static bool test_smb1_chkpath(struct cli_state *cli) return retval; } +/* + * Test BUG: https://bugzilla.samba.org/show_bug.cgi?id=15419 + */ + +static bool test_smb1_chkpath_bad(struct cli_state *cli) +{ + NTSTATUS status; + + status = smb1_chkpath(cli, "\\x//\\/"); + if (!NT_STATUS_IS_OK(status)) { + printf("%s:%d SMB1chkpath of %s failed (%s)\n", + __FILE__, + __LINE__, + "\\x//\\/", + nt_errstr(status)); + return false; + } + return true; +} + static NTSTATUS smb1_ctemp(struct cli_state *cli, const char *path, char **tmp_path) @@ -4230,3 +4250,39 @@ bool run_smb1_dfs_operations(int dummy) (void)smb1_dfs_delete(cli, "\\BAD\\BAD\\file"); return retval; } + +/* + * Test BUG: https://bugzilla.samba.org/show_bug.cgi?id=15419 + */ + +bool run_smb1_dfs_check_badpath(int dummy) +{ + struct cli_state *cli = NULL; + bool dfs_supported = false; + + printf("Starting SMB1-DFS-CHECK-BADPATH\n"); + + if (!torture_init_connection(&cli)) { + return false; + } + + if (!torture_open_connection(&cli, 0)) { + return false; + } + + /* Ensure this is a DFS share. */ + dfs_supported = smbXcli_conn_dfs_supported(cli->conn); + if (!dfs_supported) { + printf("Server %s does not support DFS\n", + smbXcli_conn_remote_name(cli->conn)); + return false; + } + dfs_supported = smbXcli_tcon_is_dfs_share(cli->smb1.tcon); + if (!dfs_supported) { + printf("Share %s does not support DFS\n", + cli->share); + return false; + } + + return test_smb1_chkpath_bad(cli); +} diff --git a/source3/torture/torture.c b/source3/torture/torture.c index c63db3f9385..8b4f456159e 100644 --- a/source3/torture/torture.c +++ b/source3/torture/torture.c @@ -15385,6 +15385,10 @@ static struct { .name = "SMB1-DFS-OPERATIONS", .fn = run_smb1_dfs_operations, }, + { + .name = "SMB1-DFS-BADPATH", + .fn = run_smb1_dfs_check_badpath, + }, { .name = "CLEANUP1", .fn = run_cleanup1, -- 2.34.1 From 7e2d56f553cd47c8e6cbae1a243430ea2cb13b53 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 26 Jul 2023 16:39:51 -0700 Subject: [PATCH 2/2] s3: smbd: Sanitize any "server" and "share" components of SMB1 DFS paths to remove UNIX separators. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove knownfail. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15419 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme Autobuild-User(master): Ralph Böhme Autobuild-Date(master): Thu Jul 27 10:52:50 UTC 2023 on atb-devel-224 (cherry picked from commit 20df26b908182f0455f301a51aeb54b6044af580) --- selftest/knownfail.d/dfs_badpath | 1 - source3/smbd/smb2_reply.c | 31 +++++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) delete mode 100644 selftest/knownfail.d/dfs_badpath diff --git a/selftest/knownfail.d/dfs_badpath b/selftest/knownfail.d/dfs_badpath deleted file mode 100644 index 9fd16e99075..00000000000 --- a/selftest/knownfail.d/dfs_badpath +++ /dev/null @@ -1 +0,0 @@ -^samba3.smbtorture_s3.smb1.SMB1-DFS-BADPATH.smbtorture\(fileserver_smb1\) diff --git a/source3/smbd/smb2_reply.c b/source3/smbd/smb2_reply.c index 76e3cf789cd..b9c3b122c06 100644 --- a/source3/smbd/smb2_reply.c +++ b/source3/smbd/smb2_reply.c @@ -335,6 +335,7 @@ static size_t srvstr_get_path_internal(TALLOC_CTX *ctx, char *share = NULL; char *remaining_path = NULL; char path_sep = 0; + char *p = NULL; if (posix_pathnames && (dst[0] == '/')) { path_sep = dst[0]; @@ -385,6 +386,16 @@ static size_t srvstr_get_path_internal(TALLOC_CTX *ctx, if (share == NULL) { goto local_path; } + /* + * Ensure the server name does not contain + * any possible path components by converting + * them to _'s. + */ + for (p = server + 1; p < share; p++) { + if (*p == '/' || *p == '\\') { + *p = '_'; + } + } /* * It's a well formed DFS path with * at least server and share components. @@ -399,6 +410,16 @@ static size_t srvstr_get_path_internal(TALLOC_CTX *ctx, */ remaining_path = strchr(share+1, path_sep); if (remaining_path == NULL) { + /* + * Ensure the share name does not contain + * any possible path components by converting + * them to _'s. + */ + for (p = share + 1; *p; p++) { + if (*p == '/' || *p == '\\') { + *p = '_'; + } + } /* * If no remaining path this was * a bare /server/share path. Just return. @@ -406,6 +427,16 @@ static size_t srvstr_get_path_internal(TALLOC_CTX *ctx, *err = NT_STATUS_OK; return ret; } + /* + * Ensure the share name does not contain + * any possible path components by converting + * them to _'s. + */ + for (p = share + 1; p < remaining_path; p++) { + if (*p == '/' || *p == '\\') { + *p = '_'; + } + } *remaining_path = '/'; dst = remaining_path + 1; /* dst now points at any following components. */ -- 2.34.1