From 35593459bc3c53b78ab98d6ce1808e5c8b070406 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 2 Dec 2021 12:05:51 -0800 Subject: [PATCH 01/92] CVE-2021-44141: s4: libcli: Add smbcli_unlink_wcard(). We will use this in place of smbcli_unlink() when we know we are using a wildcard pattern. If can be used to generally replace smbcli_unlink() as it calls down to smbcli_unlink() is no wildcard is detected. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source4/libcli/clifile.c | 96 ++++++++++++++++++++++++++++++++++++++++ source4/libcli/libcli.h | 5 +++ 2 files changed, 101 insertions(+) diff --git a/source4/libcli/clifile.c b/source4/libcli/clifile.c index 19288247c44..f013a5653bb 100644 --- a/source4/libcli/clifile.c +++ b/source4/libcli/clifile.c @@ -23,6 +23,7 @@ #include "system/filesys.h" #include "libcli/raw/libcliraw.h" #include "libcli/libcli.h" +#include "system/dir.h" /**************************************************************************** Hard/Symlink a file (UNIX extensions). @@ -148,6 +149,101 @@ NTSTATUS smbcli_unlink(struct smbcli_tree *tree, const char *fname) return smb_raw_unlink(tree, &parms); } +struct wcard_delete_state { + struct smbcli_tree *tree; + NTSTATUS status; + char *error_name; /* To help debugging. */ +}; + +static void del_fn(struct clilist_file_info *finfo, + const char *pattern, + void *priv) +{ + NTSTATUS status; + union smb_unlink parms; + char *filename = NULL; + char *dirname = NULL; + char *p = NULL; + struct wcard_delete_state *state = (struct wcard_delete_state *)priv; + + if (ISDOT(finfo->name) || ISDOTDOT(finfo->name)) { + return; + } + dirname = talloc_strdup(state, pattern); + if (dirname == NULL) { + TALLOC_FREE(state->error_name); + state->status = NT_STATUS_NO_MEMORY; + return; + } + p = strrchr_m(dirname, '\\'); + if (p != NULL) { + /* Remove the terminating '\' */ + *p = '\0'; + } + if (dirname[0] != '\0') { + filename = talloc_asprintf(dirname, + "%s\\%s", + dirname, + finfo->name); + } else { + filename = talloc_asprintf(dirname, + "%s", + finfo->name); + } + if (filename == NULL) { + TALLOC_FREE(dirname); + TALLOC_FREE(state->error_name); + state->status = NT_STATUS_NO_MEMORY; + return; + } + parms.unlink.in.pattern = filename; + parms.unlink.in.attrib = FILE_ATTRIBUTE_SYSTEM | + FILE_ATTRIBUTE_HIDDEN; + status = smb_raw_unlink(state->tree, &parms); + if (NT_STATUS_IS_OK(state->status)) { + state->status = status; + if (!NT_STATUS_IS_OK(status)) { + /* + * Save off the name we failed to + * delete to help debugging. + */ + state->error_name = talloc_move(state, &filename); + } + } + TALLOC_FREE(dirname); +} + +/**************************************************************************** + Delete a file, possibly with a wildcard pattern. +****************************************************************************/ +NTSTATUS smbcli_unlink_wcard(struct smbcli_tree *tree, const char *pattern) +{ + NTSTATUS status; + int ret; + struct wcard_delete_state *state = NULL; + + if (strchr(pattern, '*') == NULL) { + /* No wildcard, just call smbcli_unlink(). */ + return smbcli_unlink(tree, pattern); + } + state = talloc_zero(tree, struct wcard_delete_state); + if (state == NULL) { + return NT_STATUS_NO_MEMORY; + } + state->tree = tree; + ret = smbcli_list(tree, + pattern, + FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN, + del_fn, + state); + status = state->status; + TALLOC_FREE(state); + if (ret < 0) { + return NT_STATUS_UNSUCCESSFUL; + } + return status; +} + /**************************************************************************** Create a directory. ****************************************************************************/ diff --git a/source4/libcli/libcli.h b/source4/libcli/libcli.h index 652a9f5d182..9d2a3240483 100644 --- a/source4/libcli/libcli.h +++ b/source4/libcli/libcli.h @@ -158,6 +158,11 @@ NTSTATUS smbcli_rename(struct smbcli_tree *tree, const char *fname_src, ****************************************************************************/ NTSTATUS smbcli_unlink(struct smbcli_tree *tree, const char *fname); +/**************************************************************************** + Delete a wildcard pattern of files. +****************************************************************************/ +NTSTATUS smbcli_unlink_wcard(struct smbcli_tree *tree, const char *fname); + /**************************************************************************** Create a directory. ****************************************************************************/ -- 2.32.0 From 202d3dd4476bc2eef051dcf6c8124e9f37c3ca1e Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 2 Dec 2021 12:10:14 -0800 Subject: [PATCH 02/92] CVE-2021-44141: s4: libcli: In smbcli_deltree() use smbcli_unlink_wcard() in place of smbcli_unlink(). We know we have a wildcard mask here. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source4/libcli/clideltree.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source4/libcli/clideltree.c b/source4/libcli/clideltree.c index 01b33313213..3e4f9fb834f 100644 --- a/source4/libcli/clideltree.c +++ b/source4/libcli/clideltree.c @@ -119,7 +119,7 @@ int smbcli_deltree(struct smbcli_tree *tree, const char *dname) if (asprintf(&mask, "%s\\*", dname) < 0) { return -1; } - smbcli_unlink(dstate.tree, mask); + smbcli_unlink_wcard(dstate.tree, mask); smbcli_list(dstate.tree, mask, FILE_ATTRIBUTE_DIRECTORY|FILE_ATTRIBUTE_HIDDEN|FILE_ATTRIBUTE_SYSTEM, delete_fn, &dstate); -- 2.32.0 From fd88f758282b80e2b6c3f2a4ddb1b01a671474d4 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 2 Dec 2021 12:08:49 -0800 Subject: [PATCH 03/92] CVE-2021-44141: s4: torture: In raw.notify test use smbcli_unlink_wcard() in place of smbcli_unlink(). We know we have a wildcard mask here. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source4/torture/raw/notify.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source4/torture/raw/notify.c b/source4/torture/raw/notify.c index 881ad6b71cc..f3c38068780 100644 --- a/source4/torture/raw/notify.c +++ b/source4/torture/raw/notify.c @@ -254,7 +254,7 @@ static bool test_notify_dir(struct torture_context *tctx, torture_comment(tctx, "Testing notify on wildcard unlink for %d files\n", count-1); /* (2nd unlink) do a wildcard unlink */ - status = smbcli_unlink(cli2->tree, BASEDIR_CN1_DIR "\\test*.txt"); + status = smbcli_unlink_wcard(cli2->tree, BASEDIR_CN1_DIR "\\test*.txt"); torture_assert_ntstatus_ok_goto(tctx, status, ret, done, "smb_raw_changenotify_recv"); -- 2.32.0 From 3fcd599c48f48fbdb32972f680439b5a80013ae5 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 1 Dec 2021 17:52:37 -0800 Subject: [PATCH 04/92] CVE-2021-44141: s4: torture: Use smbcli_unlink_wcard() to remove wildcards in base.chkpath test. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source4/torture/basic/base.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source4/torture/basic/base.c b/source4/torture/basic/base.c index e91fef1d6c2..232ba9c5cb3 100644 --- a/source4/torture/basic/base.c +++ b/source4/torture/basic/base.c @@ -1461,7 +1461,7 @@ static bool torture_chkpath_test(struct torture_context *tctx, /* cleanup from an old run */ smbcli_rmdir(cli->tree, "\\chkpath.dir\\dir2"); - smbcli_unlink(cli->tree, "\\chkpath.dir\\*"); + smbcli_unlink_wcard(cli->tree, "\\chkpath.dir\\*"); smbcli_rmdir(cli->tree, "\\chkpath.dir"); if (NT_STATUS_IS_ERR(smbcli_mkdir(cli->tree, "\\chkpath.dir"))) { @@ -1516,7 +1516,7 @@ static bool torture_chkpath_test(struct torture_context *tctx, } smbcli_rmdir(cli->tree, "\\chkpath.dir\\dir2"); - smbcli_unlink(cli->tree, "\\chkpath.dir\\*"); + smbcli_unlink_wcard(cli->tree, "\\chkpath.dir\\*"); smbcli_rmdir(cli->tree, "\\chkpath.dir"); return ret; -- 2.32.0 From e8c4afc434fd56256a1ddd216650520f8bcb572f Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 1 Dec 2021 17:58:58 -0800 Subject: [PATCH 05/92] CVE-2021-44141: s4: torture: Use smbcli_unlink_wcard() to cleanup in base.mangle test. Avoid using smbcli_unlink() calls with wildcard names. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source4/torture/basic/mangle_test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source4/torture/basic/mangle_test.c b/source4/torture/basic/mangle_test.c index 0c3ccd54bf9..9bd3cf55dfb 100644 --- a/source4/torture/basic/mangle_test.c +++ b/source4/torture/basic/mangle_test.c @@ -195,7 +195,7 @@ bool torture_mangle(struct torture_context *torture, } } - smbcli_unlink(cli->tree, "\\mangle_test\\*"); + smbcli_unlink_wcard(cli->tree, "\\mangle_test\\*"); if (NT_STATUS_IS_ERR(smbcli_rmdir(cli->tree, "\\mangle_test"))) { printf("ERROR: Failed to remove directory\n"); return false; -- 2.32.0 From bf71556706d5aaca92a2100c4d7b12b31a752c9c Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 1 Dec 2021 18:03:57 -0800 Subject: [PATCH 06/92] CVE-2021-44141: s4: torture: Use smbcli_unlink_wcard() in base.casetable test. Avoid smbcli_unlink() calls with a wildcard path. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source4/torture/basic/utable.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source4/torture/basic/utable.c b/source4/torture/basic/utable.c index da2fe2e0b37..a3ddf1a7621 100644 --- a/source4/torture/basic/utable.c +++ b/source4/torture/basic/utable.c @@ -195,7 +195,7 @@ bool torture_casetable(struct torture_context *tctx, smbcli_close(cli->tree, fnum); } - smbcli_unlink(cli->tree, "\\utable\\*"); + smbcli_unlink_wcard(cli->tree, "\\utable\\*"); smbcli_rmdir(cli->tree, "\\utable"); return true; -- 2.32.0 From 8f3afcf08f3f31d72d9e1429e083abd4400fc93b Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 1 Dec 2021 18:08:32 -0800 Subject: [PATCH 07/92] CVE-2021-44141: s4: torture: Use smbcli_unlink_wcard() to setup and cleanup in masktest. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source4/torture/masktest.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source4/torture/masktest.c b/source4/torture/masktest.c index 5b2457a92e9..e311769a43d 100644 --- a/source4/torture/masktest.c +++ b/source4/torture/masktest.c @@ -225,7 +225,7 @@ static void test_mask(int argc, char *argv[], smbcli_mkdir(cli->tree, "\\masktest"); - smbcli_unlink(cli->tree, "\\masktest\\*"); + smbcli_unlink_wcard(cli->tree, "\\masktest\\*"); if (argc >= 2) { while (argc >= 2) { -- 2.32.0 From 35aa14e07f7ee7304685b06fcab98101f7a61345 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 2 Dec 2021 14:23:10 -0800 Subject: [PATCH 08/92] CVE-2021-44141: s4: libcli: smbcli_unlink() is no longer used with wildcard patterns. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source4/libcli/clifile.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/source4/libcli/clifile.c b/source4/libcli/clifile.c index f013a5653bb..c6a5cd5a5e7 100644 --- a/source4/libcli/clifile.c +++ b/source4/libcli/clifile.c @@ -140,11 +140,7 @@ NTSTATUS smbcli_unlink(struct smbcli_tree *tree, const char *fname) union smb_unlink parms; parms.unlink.in.pattern = fname; - if (strchr(fname, '*')) { - parms.unlink.in.attrib = FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN; - } else { - parms.unlink.in.attrib = FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN | FILE_ATTRIBUTE_DIRECTORY; - } + parms.unlink.in.attrib = FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN | FILE_ATTRIBUTE_DIRECTORY; return smb_raw_unlink(tree, &parms); } -- 2.32.0 From 25057d960b3cbfe4464c7543b67eb480056a08e2 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 2 Dec 2021 13:47:07 -0800 Subject: [PATCH 09/92] CVE-2021-44141: s3: torture: Add torture_deltree() for setup and teardown. Not yet used. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source3/torture/proto.h | 1 + source3/torture/torture.c | 127 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 128 insertions(+) diff --git a/source3/torture/proto.h b/source3/torture/proto.h index 65fa17523d8..d4db60f9dde 100644 --- a/source3/torture/proto.h +++ b/source3/torture/proto.h @@ -74,6 +74,7 @@ bool torture_ioctl_test(int dummy); bool torture_chkpath_test(int dummy); NTSTATUS torture_setup_unix_extensions(struct cli_state *cli); void torture_conn_set_sockopt(struct cli_state *cli); +void torture_deltree(struct cli_state *cli, const char *dname); /* The following definitions come from torture/utable.c */ diff --git a/source3/torture/torture.c b/source3/torture/torture.c index 4a886614ae1..a995b54df9c 100644 --- a/source3/torture/torture.c +++ b/source3/torture/torture.c @@ -458,6 +458,133 @@ void torture_conn_set_sockopt(struct cli_state *cli) smbXcli_conn_set_sockopt(cli->conn, sockops); } +static NTSTATUS torture_delete_fn(struct file_info *finfo, + const char *pattern, + void *state) +{ + NTSTATUS status; + char *filename = NULL; + char *dirname = NULL; + char *p = NULL; + TALLOC_CTX *frame = talloc_stackframe(); + struct cli_state *cli = (struct cli_state *)state; + + if (ISDOT(finfo->name) || ISDOTDOT(finfo->name)) { + TALLOC_FREE(frame); + return NT_STATUS_OK; + } + + dirname = talloc_strdup(frame, pattern); + if (dirname == NULL) { + TALLOC_FREE(frame); + return NT_STATUS_NO_MEMORY; + } + p = strrchr_m(dirname, '\\'); + if (p != NULL) { + /* Remove the terminating '\' */ + *p = '\0'; + } + if (dirname[0] != '\0') { + filename = talloc_asprintf(frame, + "%s\\%s", + dirname, + finfo->name); + } else { + filename = talloc_asprintf(frame, + "%s", + finfo->name); + } + if (filename == NULL) { + TALLOC_FREE(frame); + return NT_STATUS_NO_MEMORY; + } + if (finfo->attr & FILE_ATTRIBUTE_DIRECTORY) { + char *subdirname = talloc_asprintf(frame, + "%s\\*", + filename); + if (subdirname == NULL) { + TALLOC_FREE(frame); + return NT_STATUS_NO_MEMORY; + } + status = cli_list(cli, + subdirname, + FILE_ATTRIBUTE_DIRECTORY | + FILE_ATTRIBUTE_HIDDEN | + FILE_ATTRIBUTE_SYSTEM, + torture_delete_fn, + cli); + if (NT_STATUS_IS_OK(status)) { + printf("torture_delete_fn: cli_list " + "of %s failed (%s)\n", + subdirname, + nt_errstr(status)); + TALLOC_FREE(frame); + return status; + } + status = cli_rmdir(cli, filename); + } else { + status = cli_unlink(cli, + filename, + FILE_ATTRIBUTE_SYSTEM | + FILE_ATTRIBUTE_HIDDEN); + } + if (!NT_STATUS_IS_OK(status)) { + if (finfo->attr & FILE_ATTRIBUTE_DIRECTORY) { + printf("torture_delete_fn: cli_rmdir" + " of %s failed (%s)\n", + filename, + nt_errstr(status)); + } else { + printf("torture_delete_fn: cli_unlink" + " of %s failed (%s)\n", + filename, + nt_errstr(status)); + } + } + TALLOC_FREE(frame); + return status; +} + +void torture_deltree(struct cli_state *cli, const char *dname) +{ + char *mask = NULL; + NTSTATUS status; + + /* It might be a file */ + (void)cli_unlink(cli, + dname, + FILE_ATTRIBUTE_SYSTEM | + FILE_ATTRIBUTE_HIDDEN); + + mask = talloc_asprintf(cli, + "%s\\*", + dname); + if (mask == NULL) { + printf("torture_deltree: talloc_asprintf failed\n"); + return; + } + + status = cli_list(cli, + mask, + FILE_ATTRIBUTE_DIRECTORY | + FILE_ATTRIBUTE_HIDDEN| + FILE_ATTRIBUTE_SYSTEM, + torture_delete_fn, + cli); + if (!NT_STATUS_IS_OK(status)) { + printf("torture_deltree: cli_list of %s failed (%s)\n", + mask, + nt_errstr(status)); + } + TALLOC_FREE(mask); + status = cli_rmdir(cli, dname); + if (!NT_STATUS_IS_OK(status)) { + printf("torture_deltree: cli_rmdir of %s failed (%s)\n", + dname, + nt_errstr(status)); + } +} + /* check if the server produced the expected dos or nt error code */ static bool check_both_error(int line, NTSTATUS status, uint8_t eclass, uint32_t ecode, NTSTATUS nterr) -- 2.32.0 From 4692868aaab3f5adb7f3f111dcf07f4fdc4c24e8 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 1 Dec 2021 12:51:54 -0800 Subject: [PATCH 10/92] CVE-2021-44141: s3: torture: In run_smb1_wild_mangle_unlink_test() use torture_deltree() for setup and cleanup. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source3/torture/torture.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/source3/torture/torture.c b/source3/torture/torture.c index a995b54df9c..cb11524446a 100644 --- a/source3/torture/torture.c +++ b/source3/torture/torture.c @@ -12814,10 +12814,7 @@ static bool run_smb1_wild_mangle_unlink_test(int dummy) } /* Start fresh. */ - cli_unlink(cli, - star_name, - FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN); - cli_rmdir(cli, dname); + torture_deltree(cli, dname); /* * Create two files - 'a' and '*'. @@ -12927,10 +12924,7 @@ static bool run_smb1_wild_mangle_unlink_test(int dummy) TALLOC_FREE(mangled_name); if (cli != NULL) { - cli_unlink(cli, - star_name, - FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN); - cli_rmdir(cli, dname); + torture_deltree(cli, dname); torture_close_connection(cli); } -- 2.32.0 From 6c57d93492f681984697ea41a4a1689b4e84ace6 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 1 Dec 2021 13:51:12 -0800 Subject: [PATCH 11/92] CVE-2021-44141: s3: torture: In run_smb1_wild_mangle_rename_test() use torture_deltree() for setup and cleanup. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source3/torture/torture.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/source3/torture/torture.c b/source3/torture/torture.c index cb11524446a..5b5b5287d55 100644 --- a/source3/torture/torture.c +++ b/source3/torture/torture.c @@ -12972,10 +12972,7 @@ static bool run_smb1_wild_mangle_rename_test(int dummy) smbXcli_conn_set_sockopt(cli->conn, sockops); /* Ensure we start from fresh. */ - cli_unlink(cli, - wild_name, - FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN); - cli_posix_rmdir(cli_posix, dname); + torture_deltree(cli, dname); /* * Create two files - 'foo' and 'fo*'. @@ -13094,13 +13091,10 @@ static bool run_smb1_wild_mangle_rename_test(int dummy) TALLOC_FREE(windows_rename_src); if (cli != NULL) { - cli_unlink(cli, - wild_name, - FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN); + torture_deltree(cli, dname); torture_close_connection(cli); } - cli_posix_rmdir(cli_posix, dname); torture_close_connection(cli_posix); return correct; -- 2.32.0 From b55257f44f5e79da814a2ae02924a44d0752d20e Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 2 Dec 2021 14:13:41 -0800 Subject: [PATCH 12/92] CVE-2021-44141: s3: torture: In torture_utable(), use torture_deltree() for setup. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source3/torture/utable.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source3/torture/utable.c b/source3/torture/utable.c index a912c4cb3b0..01f6b2e51e9 100644 --- a/source3/torture/utable.c +++ b/source3/torture/utable.c @@ -43,8 +43,8 @@ bool torture_utable(int dummy) memset(valid, 0, sizeof(valid)); + torture_deltree(cli, "\\utable"); cli_mkdir(cli, "\\utable"); - cli_unlink(cli, "\\utable\\*", FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN); for (c=1; c < 0x10000; c++) { size_t size = 0; -- 2.32.0 From b74ad9e2ef47064eadb225a4387bdd7511c51369 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 2 Dec 2021 14:14:53 -0800 Subject: [PATCH 13/92] CVE-2021-44141: s3: torture: In torture_casetable(), use torture_deltree() for setup and cleanup. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source3/torture/utable.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/source3/torture/utable.c b/source3/torture/utable.c index 01f6b2e51e9..059cae9edd1 100644 --- a/source3/torture/utable.c +++ b/source3/torture/utable.c @@ -146,8 +146,7 @@ bool torture_casetable(int dummy) memset(equiv, 0, sizeof(equiv)); - cli_unlink(cli, "\\utable\\*", FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN); - cli_rmdir(cli, "\\utable"); + torture_deltree(cli, "\\utable"); if (!NT_STATUS_IS_OK(cli_mkdir(cli, "\\utable"))) { printf("Failed to create utable directory!\n"); return False; @@ -205,8 +204,7 @@ bool torture_casetable(int dummy) cli_close(cli, fnum); } - cli_unlink(cli, "\\utable\\*", FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN); - cli_rmdir(cli, "\\utable"); + torture_deltree(cli, "\\utable"); return True; } -- 2.32.0 From 718831a0f430a1a3ddc0757420c11f9c0134e02d Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 2 Dec 2021 14:16:38 -0800 Subject: [PATCH 14/92] CVE-2021-44141: s3: torture: In torture_chkpath_test(), use torture_deltree() for setup and cleanup. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source3/torture/torture.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/source3/torture/torture.c b/source3/torture/torture.c index 5b5b5287d55..e4343cf3050 100644 --- a/source3/torture/torture.c +++ b/source3/torture/torture.c @@ -9933,9 +9933,7 @@ bool torture_chkpath_test(int dummy) printf("starting chkpath test\n"); /* cleanup from an old run */ - cli_rmdir(cli, "\\chkpath.dir\\dir2"); - cli_unlink(cli, "\\chkpath.dir\\*", FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN); - cli_rmdir(cli, "\\chkpath.dir"); + torture_deltree(cli, "\\chkpath.dir"); status = cli_mkdir(cli, "\\chkpath.dir"); if (!NT_STATUS_IS_OK(status)) { @@ -9996,9 +9994,7 @@ bool torture_chkpath_test(int dummy) ret = False; } - cli_rmdir(cli, "\\chkpath.dir\\dir2"); - cli_unlink(cli, "\\chkpath.dir\\*", FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN); - cli_rmdir(cli, "\\chkpath.dir"); + torture_deltree(cli, "\\chkpath.dir"); if (!torture_close_connection(cli)) { return False; -- 2.32.0 From e78813b03220079fcf2a49d7f8b3f20d1642171b Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 2 Dec 2021 14:18:56 -0800 Subject: [PATCH 15/92] CVE-2021-44141: s3: torture: In run_streamerror(), use torture_deltree() for setup. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source3/torture/torture.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/source3/torture/torture.c b/source3/torture/torture.c index e4343cf3050..9ad0f2120e5 100644 --- a/source3/torture/torture.c +++ b/source3/torture/torture.c @@ -12464,8 +12464,7 @@ static bool run_streamerror(int dummy) return false; } - cli_unlink(cli, "\\testdir_streamerror\\*", FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN); - cli_rmdir(cli, dname); + torture_deltree(cli, dname); status = cli_mkdir(cli, dname); if (!NT_STATUS_IS_OK(status)) { -- 2.32.0 From d2ce8098019c2b83f6f1840fb3201aa804b1057c Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 2 Dec 2021 14:20:07 -0800 Subject: [PATCH 16/92] CVE-2021-44141: s3: torture: In test_mask(), use torture_deltree() for setup. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source3/torture/masktest.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/source3/torture/masktest.c b/source3/torture/masktest.c index ebd156167cd..d7485cfb5eb 100644 --- a/source3/torture/masktest.c +++ b/source3/torture/masktest.c @@ -355,10 +355,9 @@ static void test_mask(int argc, char *argv[], int fc_len = strlen(filechars); TALLOC_CTX *ctx = talloc_tos(); + torture_deltree(cli, "\\masktest"); cli_mkdir(cli, "\\masktest"); - cli_unlink(cli, "\\masktest\\*", FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN); - if (argc >= 2) { while (argc >= 2) { mask = talloc_asprintf(ctx, -- 2.32.0 From a958e457c63e15ff6619775ead031ca466c0850f Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 2 Dec 2021 14:21:47 -0800 Subject: [PATCH 17/92] CVE-2021-44141: s3: torture: In torture_mangle(), use torture_deltree() for setup and cleanup. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source3/torture/mangle_test.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/source3/torture/mangle_test.c b/source3/torture/mangle_test.c index 5832a92cdda..92754b9eeb6 100644 --- a/source3/torture/mangle_test.c +++ b/source3/torture/mangle_test.c @@ -188,8 +188,7 @@ bool torture_mangle(int dummy) return False; } - cli_unlink(cli, "\\mangle_test\\*", FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN); - cli_rmdir(cli, "\\mangle_test"); + torture_deltree(cli, "\\mangle_test"); if (!NT_STATUS_IS_OK(cli_mkdir(cli, "\\mangle_test"))) { printf("ERROR: Failed to make directory\n"); @@ -212,11 +211,7 @@ bool torture_mangle(int dummy) } } - cli_unlink(cli, "\\mangle_test\\*", FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN); - if (!NT_STATUS_IS_OK(cli_rmdir(cli, "\\mangle_test"))) { - printf("ERROR: Failed to remove directory\n"); - return False; - } + torture_deltree(cli, "\\mangle_test"); printf("\nTotal collisions %u/%u - %.2f%% (%u failures)\n", collisions, total, (100.0*collisions) / total, failures); -- 2.32.0 From 52a5579a41377ef21f22507d9ea98f5dcb5d8fa6 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 2 Dec 2021 14:10:41 -0800 Subject: [PATCH 18/92] CVE-2021-44141: s3: torture: In run_smb1_wild_mangle_unlink_test() use a valid pathname for rename target. The server will not be supporting wildcard rename soon. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source3/torture/torture.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source3/torture/torture.c b/source3/torture/torture.c index 9ad0f2120e5..b80a1113a5f 100644 --- a/source3/torture/torture.c +++ b/source3/torture/torture.c @@ -12941,7 +12941,7 @@ static bool run_smb1_wild_mangle_rename_test(int dummy) const char *foostar_name = "smb1_wild_mangle_rename/fo*"; const char *wild_name = "smb1_wild_mangle_rename/*"; char *windows_rename_src = NULL; - const char *windows_rename_dst = "smb1_wild_mangle_rename\\ba*"; + const char *windows_rename_dst = "smb1_wild_mangle_rename\\bar"; char *mangled_name = NULL; NTSTATUS status; -- 2.32.0 From dc99d2d5596b502ca50b8a16dc6e718af02714e5 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 1 Dec 2021 12:05:20 -0800 Subject: [PATCH 19/92] CVE-2021-44141: s4: torture: Remove the wildcard unlink test code. This is pre WindowXP SMB1 functionality, and we need to remove this from the server in order to move towards SMB2-only, so the test must go. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source4/torture/raw/unlink.c | 72 ------------------------------------ 1 file changed, 72 deletions(-) diff --git a/source4/torture/raw/unlink.c b/source4/torture/raw/unlink.c index 4f198fa2759..53059aa576a 100644 --- a/source4/torture/raw/unlink.c +++ b/source4/torture/raw/unlink.c @@ -112,78 +112,6 @@ static bool test_unlink(struct torture_context *tctx, struct smbcli_state *cli) status = smb_raw_unlink(cli->tree, &io); CHECK_STATUS(status, NT_STATUS_FILE_IS_A_DIRECTORY); - printf("Trying wildcards\n"); - smbcli_close(cli->tree, smbcli_open(cli->tree, fname, O_RDWR|O_CREAT, DENY_NONE)); - io.unlink.in.pattern = BASEDIR "\\t*.t"; - io.unlink.in.attrib = 0; - status = smb_raw_unlink(cli->tree, &io); - CHECK_STATUS(status, NT_STATUS_NO_SUCH_FILE); - - io.unlink.in.pattern = BASEDIR "\\z*"; - io.unlink.in.attrib = 0; - status = smb_raw_unlink(cli->tree, &io); - CHECK_STATUS(status, NT_STATUS_NO_SUCH_FILE); - - io.unlink.in.pattern = BASEDIR "\\z*"; - io.unlink.in.attrib = FILE_ATTRIBUTE_DIRECTORY; - status = smb_raw_unlink(cli->tree, &io); - - if (torture_setting_bool(tctx, "samba3", false)) { - /* - * In Samba3 we gave up upon getting the error codes in - * wildcard unlink correct. Trying gentest showed that this is - * irregular beyond our capabilities. So for - * FILE_ATTRIBUTE_DIRECTORY we always return NAME_INVALID. - * Tried by jra and vl. If others feel like solving this - * puzzle, please tell us :-) - */ - CHECK_STATUS(status, NT_STATUS_OBJECT_NAME_INVALID); - } - else { - CHECK_STATUS(status, NT_STATUS_NO_SUCH_FILE); - } - - io.unlink.in.pattern = BASEDIR "\\*"; - io.unlink.in.attrib = FILE_ATTRIBUTE_DIRECTORY; - status = smb_raw_unlink(cli->tree, &io); - CHECK_STATUS(status, NT_STATUS_OBJECT_NAME_INVALID); - - io.unlink.in.pattern = BASEDIR "\\?"; - io.unlink.in.attrib = FILE_ATTRIBUTE_DIRECTORY; - status = smb_raw_unlink(cli->tree, &io); - CHECK_STATUS(status, NT_STATUS_OBJECT_NAME_INVALID); - - io.unlink.in.pattern = BASEDIR "\\t*"; - io.unlink.in.attrib = FILE_ATTRIBUTE_DIRECTORY; - status = smb_raw_unlink(cli->tree, &io); - if (torture_setting_bool(tctx, "samba3", false)) { - CHECK_STATUS(status, NT_STATUS_OBJECT_NAME_INVALID); - } - else { - CHECK_STATUS(status, NT_STATUS_OK); - } - - smbcli_close(cli->tree, smbcli_open(cli->tree, fname, O_RDWR|O_CREAT, DENY_NONE)); - - io.unlink.in.pattern = BASEDIR "\\*.dat"; - io.unlink.in.attrib = FILE_ATTRIBUTE_DIRECTORY; - status = smb_raw_unlink(cli->tree, &io); - if (torture_setting_bool(tctx, "samba3", false)) { - CHECK_STATUS(status, NT_STATUS_OBJECT_NAME_INVALID); - } - else { - CHECK_STATUS(status, NT_STATUS_NO_SUCH_FILE); - } - - io.unlink.in.pattern = BASEDIR "\\*.tx?"; - io.unlink.in.attrib = 0; - status = smb_raw_unlink(cli->tree, &io); - CHECK_STATUS(status, NT_STATUS_OK); - - status = smb_raw_unlink(cli->tree, &io); - CHECK_STATUS(status, NT_STATUS_NO_SUCH_FILE); - - done: smb_raw_exit(cli->session); smbcli_deltree(cli->tree, BASEDIR); -- 2.32.0 From 1827c0bf8b497089401ea45cc1988d5663591c8c Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 1 Dec 2021 13:22:39 -0800 Subject: [PATCH 20/92] CVE-2021-44141: s4: torture: Remove the wildcard rename test code. This is pre WindowXP SMB1 functionality, and we need to remove this from the server in order to move towards SMB2-only, so the test must go. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source4/torture/raw/rename.c | 33 --------------------------------- 1 file changed, 33 deletions(-) diff --git a/source4/torture/raw/rename.c b/source4/torture/raw/rename.c index 63e13c0546f..5f48c055984 100644 --- a/source4/torture/raw/rename.c +++ b/source4/torture/raw/rename.c @@ -146,39 +146,6 @@ static bool test_mv(struct torture_context *tctx, status = smb_raw_rename(cli->tree, &io); CHECK_STATUS(status, NT_STATUS_OBJECT_NAME_NOT_FOUND); - - torture_comment(tctx, "trying wildcard rename\n"); - io.rename.in.pattern1 = BASEDIR "\\*.txt"; - io.rename.in.pattern2 = fname1; - - status = smb_raw_rename(cli->tree, &io); - CHECK_STATUS(status, NT_STATUS_OK); - - torture_comment(tctx, "and again\n"); - status = smb_raw_rename(cli->tree, &io); - CHECK_STATUS(status, NT_STATUS_OK); - - torture_comment(tctx, "Trying extension change\n"); - io.rename.in.pattern1 = BASEDIR "\\*.txt"; - io.rename.in.pattern2 = BASEDIR "\\*.bak"; - status = smb_raw_rename(cli->tree, &io); - CHECK_STATUS(status, NT_STATUS_OK); - - status = smb_raw_rename(cli->tree, &io); - CHECK_STATUS(status, NT_STATUS_NO_SUCH_FILE); - - torture_comment(tctx, "Checking attrib handling\n"); - torture_set_file_attribute(cli->tree, BASEDIR "\\test1.bak", FILE_ATTRIBUTE_HIDDEN); - io.rename.in.pattern1 = BASEDIR "\\test1.bak"; - io.rename.in.pattern2 = BASEDIR "\\*.txt"; - io.rename.in.attrib = 0; - status = smb_raw_rename(cli->tree, &io); - CHECK_STATUS(status, NT_STATUS_NO_SUCH_FILE); - - io.rename.in.attrib = FILE_ATTRIBUTE_HIDDEN; - status = smb_raw_rename(cli->tree, &io); - CHECK_STATUS(status, NT_STATUS_OK); - done: smbcli_close(cli->tree, fnum); smb_raw_exit(cli->session); -- 2.32.0 From 97f711920f4284d16365e30485aa861e5d42392f Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 1 Dec 2021 12:18:35 -0800 Subject: [PATCH 21/92] CVE-2021-44141: s3: torture: Remove the wildcard unlink test code. This is pre WindowXP SMB1 functionality, and we need to remove this from the server in order to move towards SMB2-only, so the test must go. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- selftest/todo_smb2_tests_to_port.list | 2 - source3/selftest/tests.py | 2 +- source3/torture/torture.c | 69 --------------------------- 3 files changed, 1 insertion(+), 72 deletions(-) diff --git a/selftest/todo_smb2_tests_to_port.list b/selftest/todo_smb2_tests_to_port.list index a9d7b8b48c5..dc1df963918 100644 --- a/selftest/todo_smb2_tests_to_port.list +++ b/selftest/todo_smb2_tests_to_port.list @@ -242,7 +242,6 @@ samba3.smbtorture_s3.crypt_client.TRANS2(nt4_dc_smb1) samba3.smbtorture_s3.crypt_client.UID-REGRESSION-TEST(nt4_dc_smb1) samba3.smbtorture_s3.crypt_client.UNLINK(nt4_dc_smb1) samba3.smbtorture_s3.crypt_client.W2K(nt4_dc_smb1) -samba3.smbtorture_s3.crypt_client.WILDDELETE(nt4_dc_smb1) samba3.smbtorture_s3.crypt_client.XCOPY(nt4_dc_smb1) samba3.smbtorture_s3.crypt.POSIX-ACL-OPLOCK(nt4_dc_smb1) samba3.smbtorture_s3.crypt.POSIX-ACL-SHAREROOT(nt4_dc_smb1) @@ -327,7 +326,6 @@ samba3.smbtorture_s3.plain.TRANS2(fileserver_smb1) samba3.smbtorture_s3.plain.UID-REGRESSION-TEST(fileserver_smb1) samba3.smbtorture_s3.plain.UNLINK(fileserver_smb1) samba3.smbtorture_s3.plain.W2K(fileserver_smb1) -samba3.smbtorture_s3.plain.WILDDELETE(fileserver_smb1) samba3.smbtorture_s3.plain.WINDOWS-BAD-SYMLINK(nt4_dc_smb1) samba3.smbtorture_s3.plain.XCOPY(fileserver_smb1) samba3.smbtorture_s3.vfs_aio_fork(fileserver_smb1).RW1(fileserver_smb1) diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py index 0654d8b0495..4751e4437aa 100755 --- a/source3/selftest/tests.py +++ b/source3/selftest/tests.py @@ -113,7 +113,7 @@ fileserver_tests = [ "UNLINK", "BROWSE", "ATTR", "TRANS2", "TORTURE", "OPLOCK1", "OPLOCK2", "OPLOCK4", "STREAMERROR", "DIR", "DIR1", "DIR-CREATETIME", "TCON", "TCONDEV", "RW1", "RW2", "RW3", "LARGE_READX", "RW-SIGNING", - "OPEN", "XCOPY", "RENAME", "DELETE", "DELETE-LN", "WILDDELETE", "PROPERTIES", "W2K", + "OPEN", "XCOPY", "RENAME", "DELETE", "DELETE-LN", "PROPERTIES", "W2K", "TCON2", "IOCTL", "CHKPATH", "FDSESS", "CHAIN1", "CHAIN2", "OWNER-RIGHTS", "CHAIN3", "PIDHIGH", "CLI_SPLICE", "UID-REGRESSION-TEST", "SHORTNAME-TEST", diff --git a/source3/torture/torture.c b/source3/torture/torture.c index b80a1113a5f..0d3b01326b1 100644 --- a/source3/torture/torture.c +++ b/source3/torture/torture.c @@ -6121,71 +6121,6 @@ static bool run_delete_print_test(int dummy) return correct; } -/* - Test wildcard delete. - */ -static bool run_wild_deletetest(int dummy) -{ - struct cli_state *cli = NULL; - const char *dname = "\\WTEST"; - const char *fname = "\\WTEST\\A"; - const char *wunlink_name = "\\WTEST\\*"; - uint16_t fnum1 = (uint16_t)-1; - bool correct = false; - NTSTATUS status; - - printf("starting wildcard delete test\n"); - - if (!torture_open_connection(&cli, 0)) { - return false; - } - - smbXcli_conn_set_sockopt(cli->conn, sockops); - - cli_unlink(cli, fname, 0); - cli_rmdir(cli, dname); - status = cli_mkdir(cli, dname); - if (!NT_STATUS_IS_OK(status)) { - printf("mkdir of %s failed %s!\n", dname, nt_errstr(status)); - goto fail; - } - status = cli_openx(cli, fname, O_CREAT|O_RDONLY, DENY_NONE, &fnum1); - if (!NT_STATUS_IS_OK(status)) { - printf("open of %s failed %s!\n", fname, nt_errstr(status)); - goto fail; - } - status = cli_close(cli, fnum1); - fnum1 = -1; - - /* - * Note the unlink attribute-type of zero. This should - * map into FILE_ATTRIBUTE_NORMAL at the server even - * on a wildcard delete. - */ - - status = cli_unlink(cli, wunlink_name, 0); - if (!NT_STATUS_IS_OK(status)) { - printf("unlink of %s failed %s!\n", - wunlink_name, nt_errstr(status)); - goto fail; - } - - printf("finished wildcard delete test\n"); - - correct = true; - - fail: - - if (fnum1 != (uint16_t)-1) cli_close(cli, fnum1); - cli_unlink(cli, fname, 0); - cli_rmdir(cli, dname); - - if (cli && !torture_close_connection(cli)) { - correct = false; - } - return correct; -} - static bool run_deletetest_ln(int dummy) { struct cli_state *cli; @@ -15148,10 +15083,6 @@ static struct { .name = "DELETE-PRINT", .fn = run_delete_print_test, }, - { - .name = "WILDDELETE", - .fn = run_wild_deletetest, - }, { .name = "DELETE-LN", .fn = run_deletetest_ln, -- 2.32.0 From bddfe5a49605c45ba1955d6efb2e7c644bdca9d4 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 1 Dec 2021 12:24:07 -0800 Subject: [PATCH 22/92] CVE-2021-44141: s3: smbd: Remove support for SMBcopy SMB_COM_COPY (0x29) It's not used in our client code or tested. From MS-CIFS. This command was introduced in the LAN Manager 1.0 dialect It was rendered obsolete in the NT LAN Manager dialect. This command was used to perform server-side file copies, but is no longer used. Clients SHOULD NOT send requests using this command code. Servers receiving requests with this command code SHOULD return STATUS_NOT_IMPLEMENTED (ERRDOS/ERRbadfunc). BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source3/smbd/reply.c | 416 ++----------------------------------------- 1 file changed, 11 insertions(+), 405 deletions(-) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index f85d1122a07..faef4b23a3c 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -8766,416 +8766,22 @@ NTSTATUS copy_file(TALLOC_CTX *ctx, /**************************************************************************** Reply to a file copy. + + From MS-CIFS. + + This command was introduced in the LAN Manager 1.0 dialect + It was rendered obsolete in the NT LAN Manager dialect. + This command was used to perform server-side file copies, but + is no longer used. Clients SHOULD + NOT send requests using this command code. + Servers receiving requests with this command code + SHOULD return STATUS_NOT_IMPLEMENTED (ERRDOS/ERRbadfunc). ****************************************************************************/ void reply_copy(struct smb_request *req) { - connection_struct *conn = req->conn; - struct smb_filename *smb_fname_src = NULL; - struct smb_filename *smb_fname_src_dir = NULL; - struct smb_filename *smb_fname_dst = NULL; - char *fname_src = NULL; - char *fname_dst = NULL; - char *fname_src_mask = NULL; - char *fname_src_dir = NULL; - const char *p; - int count=0; - int error = ERRnoaccess; - int tid2; - int ofun; - int flags; - bool target_is_directory=False; - bool source_has_wild = False; - bool dest_has_wild = False; - NTSTATUS status; - uint32_t ucf_flags_src = UCF_ALWAYS_ALLOW_WCARD_LCOMP | - ucf_flags_from_smb_request(req); - uint32_t ucf_flags_dst = UCF_ALWAYS_ALLOW_WCARD_LCOMP | - ucf_flags_from_smb_request(req); - TALLOC_CTX *ctx = talloc_tos(); - START_PROFILE(SMBcopy); - - if (req->wct < 3) { - reply_nterror(req, NT_STATUS_INVALID_PARAMETER); - goto out; - } - - tid2 = SVAL(req->vwv+0, 0); - ofun = SVAL(req->vwv+1, 0); - flags = SVAL(req->vwv+2, 0); - - p = (const char *)req->buf; - p += srvstr_get_path_req(ctx, req, &fname_src, p, STR_TERMINATE, - &status); - if (!NT_STATUS_IS_OK(status)) { - reply_nterror(req, status); - goto out; - } - p += srvstr_get_path_req(ctx, req, &fname_dst, p, STR_TERMINATE, - &status); - if (!NT_STATUS_IS_OK(status)) { - reply_nterror(req, status); - goto out; - } - - DEBUG(3,("reply_copy : %s -> %s\n", fname_src, fname_dst)); - - if (tid2 != conn->cnum) { - /* can't currently handle inter share copies XXXX */ - DEBUG(3,("Rejecting inter-share copy\n")); - reply_nterror(req, NT_STATUS_BAD_DEVICE_TYPE); - goto out; - } - - status = filename_convert(ctx, conn, - fname_src, - ucf_flags_src, - 0, - &smb_fname_src); - if (!NT_STATUS_IS_OK(status)) { - if (NT_STATUS_EQUAL(status,NT_STATUS_PATH_NOT_COVERED)) { - reply_botherror(req, NT_STATUS_PATH_NOT_COVERED, - ERRSRV, ERRbadpath); - goto out; - } - reply_nterror(req, status); - goto out; - } - - status = filename_convert(ctx, conn, - fname_dst, - ucf_flags_dst, - 0, - &smb_fname_dst); - if (!NT_STATUS_IS_OK(status)) { - if (NT_STATUS_EQUAL(status,NT_STATUS_PATH_NOT_COVERED)) { - reply_botherror(req, NT_STATUS_PATH_NOT_COVERED, - ERRSRV, ERRbadpath); - goto out; - } - reply_nterror(req, status); - goto out; - } - - target_is_directory = VALID_STAT_OF_DIR(smb_fname_dst->st); - - if ((flags&1) && target_is_directory) { - reply_nterror(req, NT_STATUS_NO_SUCH_FILE); - goto out; - } - - if ((flags&2) && !target_is_directory) { - reply_nterror(req, NT_STATUS_OBJECT_PATH_NOT_FOUND); - goto out; - } - - if ((flags&(1<<5)) && VALID_STAT_OF_DIR(smb_fname_src->st)) { - /* wants a tree copy! XXXX */ - DEBUG(3,("Rejecting tree copy\n")); - reply_nterror(req, NT_STATUS_INVALID_PARAMETER); - goto out; - } - - /* Split up the directory from the filename/mask. */ - status = split_fname_dir_mask(ctx, smb_fname_src->base_name, - &fname_src_dir, &fname_src_mask); - if (!NT_STATUS_IS_OK(status)) { - reply_nterror(req, NT_STATUS_NO_MEMORY); - goto out; - } - - if (!req->posix_pathnames) { - char *orig_src_lcomp = NULL; - char *orig_dst_lcomp = NULL; - /* - * Check the wildcard mask *before* - * unmangling. As mangling is done - * for names that can't be returned - * to Windows the unmangled name may - * contain Windows wildcard characters. - */ - orig_src_lcomp = get_original_lcomp(ctx, - conn, - fname_src, - ucf_flags_src); - if (orig_src_lcomp == NULL) { - reply_nterror(req, NT_STATUS_NO_MEMORY); - goto out; - } - orig_dst_lcomp = get_original_lcomp(ctx, - conn, - fname_dst, - ucf_flags_dst); - if (orig_dst_lcomp == NULL) { - reply_nterror(req, NT_STATUS_NO_MEMORY); - goto out; - } - source_has_wild = ms_has_wild(orig_src_lcomp); - dest_has_wild = ms_has_wild(orig_dst_lcomp); - TALLOC_FREE(orig_src_lcomp); - TALLOC_FREE(orig_dst_lcomp); - } - - /* - * We should only check the mangled cache - * here if unix_convert failed. This means - * that the path in 'mask' doesn't exist - * on the file system and so we need to look - * for a possible mangle. This patch from - * Tine Smukavec . - */ - if (!VALID_STAT(smb_fname_src->st) && - mangle_is_mangled(fname_src_mask, conn->params)) { - char *new_mask = NULL; - mangle_lookup_name_from_8_3(ctx, fname_src_mask, - &new_mask, conn->params); - - /* Use demangled name if one was successfully found. */ - if (new_mask) { - TALLOC_FREE(fname_src_mask); - fname_src_mask = new_mask; - } - } - - if (!source_has_wild) { - - /* - * Only one file needs to be copied. Append the mask back onto - * the directory. - */ - TALLOC_FREE(smb_fname_src->base_name); - if (ISDOT(fname_src_dir)) { - /* Ensure we use canonical names on open. */ - smb_fname_src->base_name = talloc_asprintf(smb_fname_src, - "%s", - fname_src_mask); - } else { - smb_fname_src->base_name = talloc_asprintf(smb_fname_src, - "%s/%s", - fname_src_dir, - fname_src_mask); - } - if (!smb_fname_src->base_name) { - reply_nterror(req, NT_STATUS_NO_MEMORY); - goto out; - } - - if (dest_has_wild) { - char *fname_dst_mod = NULL; - if (!resolve_wildcards(smb_fname_dst, - smb_fname_src->base_name, - smb_fname_dst->base_name, - &fname_dst_mod)) { - reply_nterror(req, NT_STATUS_NO_MEMORY); - goto out; - } - TALLOC_FREE(smb_fname_dst->base_name); - smb_fname_dst->base_name = fname_dst_mod; - } - - status = check_name(conn, smb_fname_src); - if (!NT_STATUS_IS_OK(status)) { - reply_nterror(req, status); - goto out; - } - - status = check_name(conn, smb_fname_dst); - if (!NT_STATUS_IS_OK(status)) { - reply_nterror(req, status); - goto out; - } - - status = copy_file(ctx, conn, smb_fname_src, smb_fname_dst, - ofun, count, target_is_directory); - - if(!NT_STATUS_IS_OK(status)) { - reply_nterror(req, status); - goto out; - } else { - count++; - } - } else { - struct smb_Dir *dir_hnd = NULL; - const char *dname = NULL; - char *talloced = NULL; - long offset = 0; - - /* - * There is a wildcard that requires us to actually read the - * src dir and copy each file matching the mask to the dst. - * Right now streams won't be copied, but this could - * presumably be added with a nested loop for reach dir entry. - */ - SMB_ASSERT(!smb_fname_src->stream_name); - SMB_ASSERT(!smb_fname_dst->stream_name); - - smb_fname_src->stream_name = NULL; - smb_fname_dst->stream_name = NULL; - - if (strequal(fname_src_mask,"????????.???")) { - TALLOC_FREE(fname_src_mask); - fname_src_mask = talloc_strdup(ctx, "*"); - if (!fname_src_mask) { - reply_nterror(req, NT_STATUS_NO_MEMORY); - goto out; - } - } - - smb_fname_src_dir = synthetic_smb_fname(talloc_tos(), - fname_src_dir, - NULL, - NULL, - smb_fname_src->twrp, - smb_fname_src->flags); - if (smb_fname_src_dir == NULL) { - reply_nterror(req, NT_STATUS_NO_MEMORY); - goto out; - } - - status = check_name(conn, smb_fname_src_dir); - if (!NT_STATUS_IS_OK(status)) { - reply_nterror(req, status); - goto out; - } - - dir_hnd = OpenDir(ctx, - conn, - smb_fname_src_dir, - fname_src_mask, - 0); - if (dir_hnd == NULL) { - status = map_nt_error_from_unix(errno); - reply_nterror(req, status); - goto out; - } - - error = ERRbadfile; - - /* Iterate over the src dir copying each entry to the dst. */ - while ((dname = ReadDirName(dir_hnd, &offset, - &smb_fname_src->st, &talloced))) { - char *destname = NULL; - - if (ISDOT(dname) || ISDOTDOT(dname)) { - TALLOC_FREE(talloced); - continue; - } - - if (IS_VETO_PATH(conn, dname)) { - TALLOC_FREE(talloced); - continue; - } - - if(!mask_match(dname, fname_src_mask, - conn->case_sensitive)) { - TALLOC_FREE(talloced); - continue; - } - - error = ERRnoaccess; - - /* Get the src smb_fname struct setup. */ - TALLOC_FREE(smb_fname_src->base_name); - if (ISDOT(fname_src_dir)) { - /* Ensure we use canonical names on open. */ - smb_fname_src->base_name = - talloc_asprintf(smb_fname_src, "%s", - dname); - } else { - smb_fname_src->base_name = - talloc_asprintf(smb_fname_src, "%s/%s", - fname_src_dir, dname); - } - - if (!smb_fname_src->base_name) { - TALLOC_FREE(dir_hnd); - TALLOC_FREE(talloced); - reply_nterror(req, NT_STATUS_NO_MEMORY); - goto out; - } - - if (!resolve_wildcards(ctx, smb_fname_src->base_name, - smb_fname_dst->base_name, - &destname)) { - TALLOC_FREE(talloced); - continue; - } - if (!destname) { - TALLOC_FREE(dir_hnd); - TALLOC_FREE(talloced); - reply_nterror(req, NT_STATUS_NO_MEMORY); - goto out; - } - - TALLOC_FREE(smb_fname_dst->base_name); - smb_fname_dst->base_name = destname; - - ZERO_STRUCT(smb_fname_src->st); - vfs_stat(conn, smb_fname_src); - - status = openat_pathref_fsp(conn->cwd_fsp, - smb_fname_src); - if (!NT_STATUS_IS_OK(status)) { - DBG_INFO("openat_pathref_fsp [%s] failed: %s\n", - smb_fname_str_dbg(smb_fname_src), - nt_errstr(status)); - break; - } - - if (!is_visible_fsp(smb_fname_src->fsp)) { - TALLOC_FREE(talloced); - continue; - } - - status = check_name(conn, smb_fname_src); - if (!NT_STATUS_IS_OK(status)) { - TALLOC_FREE(dir_hnd); - TALLOC_FREE(talloced); - reply_nterror(req, status); - goto out; - } - - status = check_name(conn, smb_fname_dst); - if (!NT_STATUS_IS_OK(status)) { - TALLOC_FREE(dir_hnd); - TALLOC_FREE(talloced); - reply_nterror(req, status); - goto out; - } - - DEBUG(3,("reply_copy : doing copy on %s -> %s\n", - smb_fname_src->base_name, - smb_fname_dst->base_name)); - - status = copy_file(ctx, conn, smb_fname_src, - smb_fname_dst, ofun, count, - target_is_directory); - if (NT_STATUS_IS_OK(status)) { - count++; - } - - TALLOC_FREE(talloced); - } - TALLOC_FREE(dir_hnd); - } - - if (count == 0) { - reply_nterror(req, dos_to_ntstatus(ERRDOS, error)); - goto out; - } - - reply_outbuf(req, 1, 0); - SSVAL(req->outbuf,smb_vwv0,count); - out: - TALLOC_FREE(smb_fname_src); - TALLOC_FREE(smb_fname_src_dir); - TALLOC_FREE(smb_fname_dst); - TALLOC_FREE(fname_src); - TALLOC_FREE(fname_dst); - TALLOC_FREE(fname_src_mask); - TALLOC_FREE(fname_src_dir); - + reply_nterror(req, NT_STATUS_NOT_IMPLEMENTED); END_PROFILE(SMBcopy); return; } -- 2.32.0 From dd7b608269f43aea8e27ac49e1af56814db2df48 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 1 Dec 2021 12:31:44 -0800 Subject: [PATCH 23/92] CVE-2021-44141: s3: smbd: In reply_unlink() remove the possibility of receiving a wildcard name. This was the only user of "has_wild=true" passed to unlink_internals(). Next commit will remove this functionality from unlink_internals(). BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source3/smbd/reply.c | 19 ++----------------- 1 file changed, 2 insertions(+), 17 deletions(-) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index faef4b23a3c..dd8c536ffac 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -3537,10 +3537,8 @@ void reply_unlink(struct smb_request *req) struct smb_filename *smb_fname = NULL; uint32_t dirtype; NTSTATUS status; - uint32_t ucf_flags = UCF_ALWAYS_ALLOW_WCARD_LCOMP | - ucf_flags_from_smb_request(req); + uint32_t ucf_flags = ucf_flags_from_smb_request(req); TALLOC_CTX *ctx = talloc_tos(); - bool has_wild = false; START_PROFILE(SMBunlink); @@ -3573,22 +3571,9 @@ void reply_unlink(struct smb_request *req) goto out; } - if (!req->posix_pathnames) { - char *lcomp = get_original_lcomp(ctx, - conn, - name, - ucf_flags); - if (lcomp == NULL) { - reply_nterror(req, NT_STATUS_NO_MEMORY); - goto out; - } - has_wild = ms_has_wild(lcomp); - TALLOC_FREE(lcomp); - } - DEBUG(3,("reply_unlink : %s\n", smb_fname_str_dbg(smb_fname))); - status = unlink_internals(conn, req, dirtype, smb_fname, has_wild); + status = unlink_internals(conn, req, dirtype, smb_fname, false); if (!NT_STATUS_IS_OK(status)) { if (open_was_deferred(req->xconn, req->mid)) { /* We have re-scheduled this call. */ -- 2.32.0 From 28244fc14f7decc4c240f35b7bc73eb4469a26de Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 1 Dec 2021 12:53:29 -0800 Subject: [PATCH 24/92] CVE-2021-44141: s3: smbd: Change unlink_internals() to ignore has_wild parameter. It's always passed as false now so we can remove the (horrible) enumeration code for unlink. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source3/smbd/reply.c | 226 +++++-------------------------------------- 1 file changed, 26 insertions(+), 200 deletions(-) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index dd8c536ffac..abdf5f01fd8 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -3283,11 +3283,9 @@ NTSTATUS unlink_internals(connection_struct *conn, { char *fname_dir = NULL; char *fname_mask = NULL; - int count=0; NTSTATUS status = NT_STATUS_OK; struct smb_filename *smb_fname_dir = NULL; TALLOC_CTX *ctx = talloc_tos(); - int ret; /* Split up the directory from the filename/mask. */ status = split_fname_dir_mask(ctx, smb_fname->base_name, @@ -3316,209 +3314,37 @@ NTSTATUS unlink_internals(connection_struct *conn, } } - if (!has_wild) { - - /* - * Only one file needs to be unlinked. Append the mask back - * onto the directory. - */ - TALLOC_FREE(smb_fname->base_name); - if (ISDOT(fname_dir)) { - /* Ensure we use canonical names on open. */ - smb_fname->base_name = talloc_asprintf(smb_fname, - "%s", - fname_mask); - } else { - smb_fname->base_name = talloc_asprintf(smb_fname, - "%s/%s", - fname_dir, - fname_mask); - } - if (!smb_fname->base_name) { - status = NT_STATUS_NO_MEMORY; - goto out; - } - if (dirtype == 0) { - dirtype = FILE_ATTRIBUTE_NORMAL; - } - - status = check_name(conn, smb_fname); - if (!NT_STATUS_IS_OK(status)) { - goto out; - } - - status = do_unlink(conn, req, smb_fname, dirtype); - if (!NT_STATUS_IS_OK(status)) { - goto out; - } - - count++; + /* + * Only one file needs to be unlinked. Append the mask back + * onto the directory. + */ + TALLOC_FREE(smb_fname->base_name); + if (ISDOT(fname_dir)) { + /* Ensure we use canonical names on open. */ + smb_fname->base_name = talloc_asprintf(smb_fname, + "%s", + fname_mask); } else { - struct smb_Dir *dir_hnd = NULL; - long offset = 0; - const char *dname = NULL; - char *talloced = NULL; - - if ((dirtype & SAMBA_ATTRIBUTES_MASK) == FILE_ATTRIBUTE_DIRECTORY) { - status = NT_STATUS_OBJECT_NAME_INVALID; - goto out; - } - if (dirtype == 0) { - dirtype = FILE_ATTRIBUTE_NORMAL; - } - - if (strequal(fname_mask,"????????.???")) { - TALLOC_FREE(fname_mask); - fname_mask = talloc_strdup(ctx, "*"); - if (!fname_mask) { - status = NT_STATUS_NO_MEMORY; - goto out; - } - } - - smb_fname_dir = synthetic_smb_fname(talloc_tos(), - fname_dir, - NULL, - NULL, - smb_fname->twrp, - smb_fname->flags); - if (smb_fname_dir == NULL) { - status = NT_STATUS_NO_MEMORY; - goto out; - } - - status = check_name(conn, smb_fname_dir); - if (!NT_STATUS_IS_OK(status)) { - goto out; - } - - dir_hnd = OpenDir(talloc_tos(), conn, smb_fname_dir, fname_mask, - dirtype); - if (dir_hnd == NULL) { - status = map_nt_error_from_unix(errno); - goto out; - } - - /* XXXX the CIFS spec says that if bit0 of the flags2 field is set then - the pattern matches against the long name, otherwise the short name - We don't implement this yet XXXX - */ - - status = NT_STATUS_NO_SUCH_FILE; - - while ((dname = ReadDirName(dir_hnd, &offset, - &smb_fname->st, &talloced))) { - TALLOC_CTX *frame = talloc_stackframe(); - char *p = NULL; - struct smb_filename *f = NULL; - - /* Quick check for "." and ".." */ - if (ISDOT(dname) || ISDOTDOT(dname)) { - TALLOC_FREE(frame); - TALLOC_FREE(talloced); - continue; - } - - if (IS_VETO_PATH(conn, dname)) { - TALLOC_FREE(frame); - TALLOC_FREE(talloced); - continue; - } - - if(!mask_match(dname, fname_mask, - conn->case_sensitive)) { - TALLOC_FREE(frame); - TALLOC_FREE(talloced); - continue; - } - - if (ISDOT(fname_dir)) { - /* Ensure we use canonical names on open. */ - p = talloc_asprintf(smb_fname, "%s", dname); - } else { - p = talloc_asprintf(smb_fname, "%s/%s", - fname_dir, dname); - } - if (p == NULL) { - TALLOC_FREE(dir_hnd); - status = NT_STATUS_NO_MEMORY; - TALLOC_FREE(frame); - TALLOC_FREE(talloced); - goto out; - } - f = synthetic_smb_fname(frame, - p, - NULL, - &smb_fname->st, - smb_fname->twrp, - smb_fname->flags); - if (f == NULL) { - TALLOC_FREE(dir_hnd); - status = NT_STATUS_NO_MEMORY; - TALLOC_FREE(frame); - TALLOC_FREE(talloced); - goto out; - } - - ret = vfs_stat(conn, f); - if (ret != 0) { - status = map_nt_error_from_unix(errno); - TALLOC_FREE(dir_hnd); - TALLOC_FREE(frame); - TALLOC_FREE(talloced); - goto out; - } - - status = openat_pathref_fsp(conn->cwd_fsp, f); - if (NT_STATUS_EQUAL(status, NT_STATUS_OBJECT_NAME_NOT_FOUND) && - (f->flags & SMB_FILENAME_POSIX_PATH) && - S_ISLNK(f->st.st_ex_mode)) - { - status = NT_STATUS_OK; - } - if (!NT_STATUS_IS_OK(status)) { - TALLOC_FREE(dir_hnd); - TALLOC_FREE(frame); - TALLOC_FREE(talloced); - goto out; - } - - if (!is_visible_fsp(f->fsp)) { - TALLOC_FREE(frame); - TALLOC_FREE(talloced); - continue; - } - - status = check_name(conn, f); - if (!NT_STATUS_IS_OK(status)) { - TALLOC_FREE(dir_hnd); - TALLOC_FREE(frame); - TALLOC_FREE(talloced); - goto out; - } - - status = do_unlink(conn, req, f, dirtype); - if (!NT_STATUS_IS_OK(status)) { - TALLOC_FREE(dir_hnd); - TALLOC_FREE(frame); - TALLOC_FREE(talloced); - goto out; - } - - count++; - DBG_DEBUG("successful unlink [%s]\n", - smb_fname_str_dbg(f)); - - TALLOC_FREE(frame); - TALLOC_FREE(talloced); - } - TALLOC_FREE(dir_hnd); + smb_fname->base_name = talloc_asprintf(smb_fname, + "%s/%s", + fname_dir, + fname_mask); + } + if (!smb_fname->base_name) { + status = NT_STATUS_NO_MEMORY; + goto out; + } + if (dirtype == 0) { + dirtype = FILE_ATTRIBUTE_NORMAL; } - if (count == 0 && NT_STATUS_IS_OK(status) && errno != 0) { - status = map_nt_error_from_unix(errno); + status = check_name(conn, smb_fname); + if (!NT_STATUS_IS_OK(status)) { + goto out; } + status = do_unlink(conn, req, smb_fname, dirtype); + out: TALLOC_FREE(smb_fname_dir); TALLOC_FREE(fname_dir); -- 2.32.0 From 6479e48ad8ed14952e8022f88132cf365713ce05 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 1 Dec 2021 13:03:03 -0800 Subject: [PATCH 25/92] CVE-2021-44141: s3: smbd: Remove 'bool has_wild' parameter from unlink_internals(). BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source3/printing/nt_printing.c | 2 +- source3/smbd/proto.h | 3 +-- source3/smbd/reply.c | 5 ++--- source3/smbd/trans2.c | 3 +-- 4 files changed, 5 insertions(+), 8 deletions(-) diff --git a/source3/printing/nt_printing.c b/source3/printing/nt_printing.c index b172ed92c6e..a47afda4a84 100644 --- a/source3/printing/nt_printing.c +++ b/source3/printing/nt_printing.c @@ -2042,7 +2042,7 @@ static NTSTATUS driver_unlink_internals(connection_struct *conn, goto err_out; } - status = unlink_internals(conn, NULL, 0, smb_fname, false); + status = unlink_internals(conn, NULL, 0, smb_fname); err_out: talloc_free(tmp_ctx); return status; diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h index bf7401f5191..9454d44968b 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -995,8 +995,7 @@ void reply_ctemp(struct smb_request *req); NTSTATUS unlink_internals(connection_struct *conn, struct smb_request *req, uint32_t dirtype, - struct smb_filename *smb_fname, - bool has_wcard); + struct smb_filename *smb_fname); void reply_unlink(struct smb_request *req); ssize_t fake_sendfile(struct smbXsrv_connection *xconn, files_struct *fsp, off_t startpos, size_t nread); diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index abdf5f01fd8..e996b809243 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -3278,8 +3278,7 @@ static NTSTATUS do_unlink(connection_struct *conn, NTSTATUS unlink_internals(connection_struct *conn, struct smb_request *req, uint32_t dirtype, - struct smb_filename *smb_fname, - bool has_wild) + struct smb_filename *smb_fname) { char *fname_dir = NULL; char *fname_mask = NULL; @@ -3399,7 +3398,7 @@ void reply_unlink(struct smb_request *req) DEBUG(3,("reply_unlink : %s\n", smb_fname_str_dbg(smb_fname))); - status = unlink_internals(conn, req, dirtype, smb_fname, false); + status = unlink_internals(conn, req, dirtype, smb_fname); if (!NT_STATUS_IS_OK(status)) { if (open_was_deferred(req->xconn, req->mid)) { /* We have re-scheduled this call. */ diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c index a86ac3228e3..54177aaefbe 100644 --- a/source3/smbd/trans2.c +++ b/source3/smbd/trans2.c @@ -6512,8 +6512,7 @@ NTSTATUS hardlink_internals(TALLOC_CTX *ctx, status = unlink_internals(conn, req, FILE_ATTRIBUTE_NORMAL, - smb_fname_new, - false); + smb_fname_new); if (!NT_STATUS_IS_OK(status)) { goto out; } -- 2.32.0 From 432e85c7ccfd5e83707f42c5490e2c64bd59cc48 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 1 Dec 2021 13:56:31 -0800 Subject: [PATCH 26/92] CVE-2021-44141: s3: smbd: Remove UCF_ALWAYS_ALLOW_WCARD_LCOMP flag from pathname processing in reply_mv(). We are no longer supporting wildcard rename via SMBmv (0x7) as WindowsXP SMB1 and above do not use it. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source3/smbd/reply.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index e996b809243..0bc3db4c9ca 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -8242,10 +8242,8 @@ void reply_mv(struct smb_request *req) const char *src_original_lcomp = NULL; struct smb_filename *smb_fname_dst = NULL; const char *dst_original_lcomp = NULL; - uint32_t src_ucf_flags = ucf_flags_from_smb_request(req) | - (!req->posix_pathnames ? UCF_ALWAYS_ALLOW_WCARD_LCOMP : 0); - uint32_t dst_ucf_flags = ucf_flags_from_smb_request(req) | - (!req->posix_pathnames ? UCF_ALWAYS_ALLOW_WCARD_LCOMP : 0); + uint32_t src_ucf_flags = ucf_flags_from_smb_request(req); + uint32_t dst_ucf_flags = ucf_flags_from_smb_request(req); bool stream_rename = false; START_PROFILE(SMBmv); -- 2.32.0 From 7152a0fbf312a4ce3e3f61e400efb674d9559930 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 1 Dec 2021 16:07:07 -0800 Subject: [PATCH 27/92] CVE-2021-44141: s3: smbd: In smb_file_rename_information() (SMB_FILE_RENAME_INFORMATION info level) prevent destination wildcards. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source3/smbd/trans2.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c index 54177aaefbe..f844ed8de78 100644 --- a/source3/smbd/trans2.c +++ b/source3/smbd/trans2.c @@ -7436,8 +7436,7 @@ static NTSTATUS smb_file_rename_information(connection_struct *conn, * the newname instead. */ char *base_name = NULL; - uint32_t ucf_flags = UCF_ALWAYS_ALLOW_WCARD_LCOMP| - ucf_flags_from_smb_request(req); + uint32_t ucf_flags = ucf_flags_from_smb_request(req); /* newname must *not* be a stream name. */ if (newname[0] == ':') { -- 2.32.0 From cb24ccc62357af7434c95a52257167d3c3361b9f Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 1 Dec 2021 16:08:13 -0800 Subject: [PATCH 28/92] CVE-2021-44141: s3: smbd: In SMBntrename (0xa5) prevent wildcards in destination name. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source3/smbd/nttrans.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/source3/smbd/nttrans.c b/source3/smbd/nttrans.c index ffff822e221..5e6163f0433 100644 --- a/source3/smbd/nttrans.c +++ b/source3/smbd/nttrans.c @@ -1771,6 +1771,11 @@ void reply_ntrename(struct smb_request *req) goto out; } + if (!req->posix_pathnames && ms_has_wild(newname)) { + reply_nterror(req, NT_STATUS_OBJECT_PATH_SYNTAX_BAD); + goto out; + } + if (!req->posix_pathnames) { /* The newname must begin with a ':' if the oldname contains a ':'. */ -- 2.32.0 From 386b01d305562229f743f5696d1a8d9fe87c4c55 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 1 Dec 2021 16:12:46 -0800 Subject: [PATCH 29/92] CVE-2021-44141: s3: smbd: In reply_ntrename() remove the UCF_ALWAYS_ALLOW_WCARD_LCOMP flag for destination lookups. We know the destination will never be a wildcard. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source3/smbd/nttrans.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/source3/smbd/nttrans.c b/source3/smbd/nttrans.c index 5e6163f0433..e25a59a7cb9 100644 --- a/source3/smbd/nttrans.c +++ b/source3/smbd/nttrans.c @@ -1788,14 +1788,6 @@ void reply_ntrename(struct smb_request *req) } } - /* - * If this is a rename operation, allow wildcards and save the - * destination's last component. - */ - if (rename_type == RENAME_FLAG_RENAME) { - ucf_flags_dst |= UCF_ALWAYS_ALLOW_WCARD_LCOMP; - } - /* rename_internals() calls unix_convert(), so don't call it here. */ status = filename_convert(ctx, conn, oldname, -- 2.32.0 From cb96d41a7fc0c083d14288df3f294ed53c22d60b Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 1 Dec 2021 16:14:57 -0800 Subject: [PATCH 30/92] CVE-2021-44141: s3: smbd: In reply_ntrename(), never set dest_has_wcard. It can never be true. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source3/smbd/nttrans.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/source3/smbd/nttrans.c b/source3/smbd/nttrans.c index e25a59a7cb9..f5cb82024c8 100644 --- a/source3/smbd/nttrans.c +++ b/source3/smbd/nttrans.c @@ -1816,10 +1816,6 @@ void reply_ntrename(struct smb_request *req) goto out; } - if (!req->posix_pathnames) { - dest_has_wcard = ms_has_wild(dst_original_lcomp); - } - status = filename_convert(ctx, conn, newname, ucf_flags_dst, -- 2.32.0 From 1f1cf0333f55c12ca46e73999209622e972249fb Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 1 Dec 2021 16:17:51 -0800 Subject: [PATCH 31/92] CVE-2021-44141: s3: smbd: In reply_ntrename() remove 'bool dest_has_wcard' and all uses. It's always false now. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source3/smbd/nttrans.c | 32 ++++++++++++-------------------- 1 file changed, 12 insertions(+), 20 deletions(-) diff --git a/source3/smbd/nttrans.c b/source3/smbd/nttrans.c index f5cb82024c8..b0f55ade267 100644 --- a/source3/smbd/nttrans.c +++ b/source3/smbd/nttrans.c @@ -1732,7 +1732,6 @@ void reply_ntrename(struct smb_request *req) const char *dst_original_lcomp = NULL; const char *p; NTSTATUS status; - bool dest_has_wcard = False; uint32_t attrs; uint32_t ucf_flags_src = ucf_flags_from_smb_request(req); uint32_t ucf_flags_dst = ucf_flags_from_smb_request(req); @@ -1862,27 +1861,20 @@ void reply_ntrename(struct smb_request *req) DELETE_ACCESS); break; case RENAME_FLAG_HARD_LINK: - if (dest_has_wcard) { - /* No wildcards. */ - status = NT_STATUS_OBJECT_PATH_SYNTAX_BAD; - } else { - status = hardlink_internals(ctx, conn, - req, - false, - smb_fname_old, - smb_fname_new); - } + status = hardlink_internals(ctx, + conn, + req, + false, + smb_fname_old, + smb_fname_new); break; case RENAME_FLAG_COPY: - if (dest_has_wcard) { - /* No wildcards. */ - status = NT_STATUS_OBJECT_PATH_SYNTAX_BAD; - } else { - status = copy_internals(ctx, conn, req, - smb_fname_old, - smb_fname_new, - attrs); - } + status = copy_internals(ctx, + conn, + req, + smb_fname_old, + smb_fname_new, + attrs); break; case RENAME_FLAG_MOVE_CLUSTER_INFORMATION: status = NT_STATUS_INVALID_PARAMETER; -- 2.32.0 From 399cca9b0b1bf0d29b0096458d5e99c340c1b13a Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 1 Dec 2021 16:25:03 -0800 Subject: [PATCH 32/92] CVE-2021-44141: s3: smbd: Prepare to remove wildcard matching from rename_internals(). src_has_wild and dest_has_wild can never be true. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source3/smbd/reply.c | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index 0bc3db4c9ca..3eaadb33861 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -7860,20 +7860,6 @@ NTSTATUS rename_internals(TALLOC_CTX *ctx, goto out; } - if (!(smb_fname_src->flags & SMB_FILENAME_POSIX_PATH)) { - /* - * Check the wildcard mask *before* - * unmangling. As mangling is done - * for names that can't be returned - * to Windows the unmangled name may - * contain Windows wildcard characters. - */ - if (src_original_lcomp != NULL) { - src_has_wild = ms_has_wild(src_original_lcomp); - } - dest_has_wild = ms_has_wild(dst_original_lcomp); - } - /* * We should only check the mangled cache * here if unix_convert failed. This means -- 2.32.0 From 613a0e006fd4cd1b7a71ac78a9294a0c9922578f Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 1 Dec 2021 16:26:28 -0800 Subject: [PATCH 33/92] CVE-2021-44141: s3: smbd: Remove dest_has_wild and all associated code from rename_internals() BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source3/smbd/reply.c | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index 3eaadb33861..21f40cf123e 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -7841,7 +7841,6 @@ NTSTATUS rename_internals(TALLOC_CTX *ctx, struct smb2_create_blobs *posx = NULL; int rc; bool src_has_wild = false; - bool dest_has_wild = false; /* * Split the old name into directory and last component @@ -7923,24 +7922,6 @@ NTSTATUS rename_internals(TALLOC_CTX *ctx, smb_fname_str_dbg(smb_fname_dst), dst_original_lcomp)); - /* The dest name still may have wildcards. */ - if (dest_has_wild) { - char *fname_dst_mod = NULL; - if (!resolve_wildcards(smb_fname_dst, - smb_fname_src->base_name, - smb_fname_dst->base_name, - &fname_dst_mod)) { - DEBUG(6, ("rename_internals: resolve_wildcards " - "%s %s failed\n", - smb_fname_src->base_name, - smb_fname_dst->base_name)); - status = NT_STATUS_NO_MEMORY; - goto out; - } - TALLOC_FREE(smb_fname_dst->base_name); - smb_fname_dst->base_name = fname_dst_mod; - } - ZERO_STRUCT(smb_fname_src->st); rc = vfs_stat(conn, smb_fname_src); -- 2.32.0 From aa683975f34b58d276f70618e8aec53614178f87 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 1 Dec 2021 16:29:43 -0800 Subject: [PATCH 34/92] CVE-2021-44141: s3: smbd: Remove all wildcard code from rename_internals(). We no longer use resolve_wildcards() so comment it out for later removal. Keep the '{ ... }' block around the singleton rename for now, to keep the diff small. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source3/smbd/reply.c | 201 +------------------------------------------ 1 file changed, 4 insertions(+), 197 deletions(-) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index 21f40cf123e..a0e61546323 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -7059,6 +7059,7 @@ void reply_rmdir(struct smb_request *req) return; } +#if 0 /******************************************************************* Resolve wildcards in a filename rename. ********************************************************************/ @@ -7185,6 +7186,7 @@ static bool resolve_wildcards(TALLOC_CTX *ctx, return True; } +#endif /**************************************************************************** Ensure open files have their names updated. Updated to notify other smbd's @@ -7831,24 +7833,18 @@ NTSTATUS rename_internals(TALLOC_CTX *ctx, char *fname_src_dir = NULL; struct smb_filename *smb_fname_src_dir = NULL; char *fname_src_mask = NULL; - int count=0; NTSTATUS status = NT_STATUS_OK; - struct smb_Dir *dir_hnd = NULL; - const char *dname = NULL; char *talloced = NULL; - long offset = 0; int create_options = 0; struct smb2_create_blobs *posx = NULL; int rc; - bool src_has_wild = false; /* * Split the old name into directory and last component * strings. Note that unix_convert may have stripped off a * leading ./ from both name and newname if the rename is * at the root of the share. We need to make sure either both - * name and newname contain a / character or neither of them do - * as this is checked in resolve_wildcards(). + * name and newname contain a / character or neither of them do. */ /* Split up the directory from the filename/mask. */ @@ -7888,7 +7884,7 @@ NTSTATUS rename_internals(TALLOC_CTX *ctx, } } - if (!src_has_wild) { + { files_struct *fsp; /* @@ -7992,195 +7988,6 @@ NTSTATUS rename_internals(TALLOC_CTX *ctx, nt_errstr(status), smb_fname_str_dbg(smb_fname_src), smb_fname_str_dbg(smb_fname_dst))); - goto out; - } - - /* - * Wildcards - process each file that matches. - */ - if (strequal(fname_src_mask, "????????.???")) { - TALLOC_FREE(fname_src_mask); - fname_src_mask = talloc_strdup(ctx, "*"); - if (!fname_src_mask) { - status = NT_STATUS_NO_MEMORY; - goto out; - } - } - - smb_fname_src_dir = synthetic_smb_fname(talloc_tos(), - fname_src_dir, - NULL, - NULL, - smb_fname_src->twrp, - smb_fname_src->flags); - if (smb_fname_src_dir == NULL) { - status = NT_STATUS_NO_MEMORY; - goto out; - } - - status = check_name(conn, smb_fname_src_dir); - if (!NT_STATUS_IS_OK(status)) { - goto out; - } - - dir_hnd = OpenDir(talloc_tos(), conn, smb_fname_src_dir, fname_src_mask, - attrs); - if (dir_hnd == NULL) { - status = map_nt_error_from_unix(errno); - goto out; - } - - status = NT_STATUS_NO_SUCH_FILE; - /* - * Was status = NT_STATUS_OBJECT_NAME_NOT_FOUND; - * - gentest fix. JRA - */ - - while ((dname = ReadDirName(dir_hnd, &offset, &smb_fname_src->st, - &talloced))) { - files_struct *fsp = NULL; - char *destname = NULL; - bool sysdir_entry = False; - - /* Quick check for "." and ".." */ - if (ISDOT(dname) || ISDOTDOT(dname)) { - if (attrs & FILE_ATTRIBUTE_DIRECTORY) { - sysdir_entry = True; - } else { - TALLOC_FREE(talloced); - continue; - } - } - - if(!mask_match(dname, fname_src_mask, conn->case_sensitive)) { - TALLOC_FREE(talloced); - continue; - } - - if (sysdir_entry) { - status = NT_STATUS_OBJECT_NAME_INVALID; - break; - } - - TALLOC_FREE(smb_fname_src->base_name); - if (ISDOT(fname_src_dir)) { - /* Ensure we use canonical names on open. */ - smb_fname_src->base_name = talloc_asprintf(smb_fname_src, - "%s", - dname); - } else { - smb_fname_src->base_name = talloc_asprintf(smb_fname_src, - "%s/%s", - fname_src_dir, - dname); - } - if (!smb_fname_src->base_name) { - status = NT_STATUS_NO_MEMORY; - goto out; - } - - if (!resolve_wildcards(ctx, smb_fname_src->base_name, - smb_fname_dst->base_name, - &destname)) { - DEBUG(6, ("resolve_wildcards %s %s failed\n", - smb_fname_src->base_name, destname)); - TALLOC_FREE(talloced); - continue; - } - if (!destname) { - status = NT_STATUS_NO_MEMORY; - goto out; - } - - TALLOC_FREE(smb_fname_dst->base_name); - smb_fname_dst->base_name = destname; - - ZERO_STRUCT(smb_fname_src->st); - vfs_stat(conn, smb_fname_src); - - status = openat_pathref_fsp(conn->cwd_fsp, smb_fname_src); - if (!NT_STATUS_IS_OK(status)) { - DBG_INFO("openat_pathref_fsp [%s] failed: %s\n", - smb_fname_str_dbg(smb_fname_src), - nt_errstr(status)); - break; - } - - if (!is_visible_fsp(smb_fname_src->fsp)) { - TALLOC_FREE(talloced); - continue; - } - - create_options = 0; - - if (S_ISDIR(smb_fname_src->st.st_ex_mode)) { - create_options |= FILE_DIRECTORY_FILE; - } - - status = SMB_VFS_CREATE_FILE( - conn, /* conn */ - req, /* req */ - smb_fname_src, /* fname */ - access_mask, /* access_mask */ - (FILE_SHARE_READ | /* share_access */ - FILE_SHARE_WRITE), - FILE_OPEN, /* create_disposition*/ - create_options, /* create_options */ - 0, /* file_attributes */ - 0, /* oplock_request */ - NULL, /* lease */ - 0, /* allocation_size */ - 0, /* private_flags */ - NULL, /* sd */ - NULL, /* ea_list */ - &fsp, /* result */ - NULL, /* pinfo */ - posx, /* in_context_blobs */ - NULL); /* out_context_blobs */ - - if (!NT_STATUS_IS_OK(status)) { - DEBUG(3,("rename_internals: SMB_VFS_CREATE_FILE " - "returned %s rename %s -> %s\n", - nt_errstr(status), - smb_fname_str_dbg(smb_fname_src), - smb_fname_str_dbg(smb_fname_dst))); - break; - } - - dst_original_lcomp = talloc_strdup(smb_fname_dst, dname); - if (dst_original_lcomp == NULL) { - status = NT_STATUS_NO_MEMORY; - goto out; - } - - status = rename_internals_fsp(conn, - fsp, - smb_fname_dst, - dst_original_lcomp, - attrs, - replace_if_exists); - - close_file(req, fsp, NORMAL_CLOSE); - - if (!NT_STATUS_IS_OK(status)) { - DEBUG(3, ("rename_internals_fsp returned %s for " - "rename %s -> %s\n", nt_errstr(status), - smb_fname_str_dbg(smb_fname_src), - smb_fname_str_dbg(smb_fname_dst))); - break; - } - - count++; - - DEBUG(3,("rename_internals: doing rename on %s -> " - "%s\n", smb_fname_str_dbg(smb_fname_src), - smb_fname_str_dbg(smb_fname_src))); - TALLOC_FREE(talloced); - } - TALLOC_FREE(dir_hnd); - - if (count == 0 && NT_STATUS_IS_OK(status) && errno != 0) { - status = map_nt_error_from_unix(errno); } out: -- 2.32.0 From dc025e5e5e92663eef41ca60a2a4967c9c8c8523 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 1 Dec 2021 16:31:36 -0800 Subject: [PATCH 35/92] CVE-2021-44141: s3: smbd: Remove the commented out resolve_wildcards(). BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source3/smbd/reply.c | 129 ------------------------------------------- 1 file changed, 129 deletions(-) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index a0e61546323..aec0892c81a 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -7059,135 +7059,6 @@ void reply_rmdir(struct smb_request *req) return; } -#if 0 -/******************************************************************* - Resolve wildcards in a filename rename. -********************************************************************/ - -static bool resolve_wildcards(TALLOC_CTX *ctx, - const char *name1, - const char *name2, - char **pp_newname) -{ - char *name2_copy = NULL; - char *root1 = NULL; - char *root2 = NULL; - char *ext1 = NULL; - char *ext2 = NULL; - char *p,*p2, *pname1, *pname2; - - name2_copy = talloc_strdup(ctx, name2); - if (!name2_copy) { - return False; - } - - pname1 = strrchr_m(name1,'/'); - pname2 = strrchr_m(name2_copy,'/'); - - if (!pname1 || !pname2) { - return False; - } - - /* Truncate the copy of name2 at the last '/' */ - *pname2 = '\0'; - - /* Now go past the '/' */ - pname1++; - pname2++; - - root1 = talloc_strdup(ctx, pname1); - root2 = talloc_strdup(ctx, pname2); - - if (!root1 || !root2) { - return False; - } - - p = strrchr_m(root1,'.'); - if (p) { - *p = 0; - ext1 = talloc_strdup(ctx, p+1); - } else { - ext1 = talloc_strdup(ctx, ""); - } - p = strrchr_m(root2,'.'); - if (p) { - *p = 0; - ext2 = talloc_strdup(ctx, p+1); - } else { - ext2 = talloc_strdup(ctx, ""); - } - - if (!ext1 || !ext2) { - return False; - } - - p = root1; - p2 = root2; - while (*p2) { - if (*p2 == '?') { - /* Hmmm. Should this be mb-aware ? */ - *p2 = *p; - p2++; - } else if (*p2 == '*') { - *p2 = '\0'; - root2 = talloc_asprintf(ctx, "%s%s", - root2, - p); - if (!root2) { - return False; - } - break; - } else { - p2++; - } - if (*p) { - p++; - } - } - - p = ext1; - p2 = ext2; - while (*p2) { - if (*p2 == '?') { - /* Hmmm. Should this be mb-aware ? */ - *p2 = *p; - p2++; - } else if (*p2 == '*') { - *p2 = '\0'; - ext2 = talloc_asprintf(ctx, "%s%s", - ext2, - p); - if (!ext2) { - return False; - } - break; - } else { - p2++; - } - if (*p) { - p++; - } - } - - if (*ext2) { - *pp_newname = talloc_asprintf(ctx, "%s/%s.%s", - name2_copy, - root2, - ext2); - } else { - *pp_newname = talloc_asprintf(ctx, "%s/%s", - name2_copy, - root2); - } - - if (!*pp_newname) { - return False; - } - - return True; -} -#endif - /**************************************************************************** Ensure open files have their names updated. Updated to notify other smbd's asynchronously. -- 2.32.0 From 3a4c04393d591155309be0afc98427eb83ad1d66 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 1 Dec 2021 16:35:54 -0800 Subject: [PATCH 36/92] CVE-2021-44141: s3: smbd: Inside rename_internals() remove '{ ... }' block around singleton rename code. Best viewed with 'git show -b' As we're touching the DEBUG() code, change it to modern DBG_NOTICE(). BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source3/smbd/reply.c | 132 +++++++++++++++++++++---------------------- 1 file changed, 64 insertions(+), 68 deletions(-) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index aec0892c81a..e9814037b1e 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -7708,6 +7708,7 @@ NTSTATUS rename_internals(TALLOC_CTX *ctx, char *talloced = NULL; int create_options = 0; struct smb2_create_blobs *posx = NULL; + struct files_struct *fsp = NULL; int rc; /* @@ -7755,70 +7756,67 @@ NTSTATUS rename_internals(TALLOC_CTX *ctx, } } - { - files_struct *fsp; + /* + * Only one file needs to be renamed. Append the mask back + * onto the directory. + */ + TALLOC_FREE(smb_fname_src->base_name); + if (ISDOT(fname_src_dir)) { + /* Ensure we use canonical names on open. */ + smb_fname_src->base_name = talloc_asprintf(smb_fname_src, + "%s", + fname_src_mask); + } else { + smb_fname_src->base_name = talloc_asprintf(smb_fname_src, + "%s/%s", + fname_src_dir, + fname_src_mask); + } + if (!smb_fname_src->base_name) { + status = NT_STATUS_NO_MEMORY; + goto out; + } - /* - * Only one file needs to be renamed. Append the mask back - * onto the directory. - */ - TALLOC_FREE(smb_fname_src->base_name); - if (ISDOT(fname_src_dir)) { - /* Ensure we use canonical names on open. */ - smb_fname_src->base_name = talloc_asprintf(smb_fname_src, - "%s", - fname_src_mask); - } else { - smb_fname_src->base_name = talloc_asprintf(smb_fname_src, - "%s/%s", - fname_src_dir, - fname_src_mask); - } - if (!smb_fname_src->base_name) { - status = NT_STATUS_NO_MEMORY; - goto out; - } + DBG_NOTICE("case_sensitive = %d, " + "case_preserve = %d, short case preserve = %d, " + "directory = %s, newname = %s, " + "last_component_dest = %s\n", + conn->case_sensitive, conn->case_preserve, + conn->short_case_preserve, + smb_fname_str_dbg(smb_fname_src), + smb_fname_str_dbg(smb_fname_dst), + dst_original_lcomp); - DEBUG(3, ("rename_internals: case_sensitive = %d, " - "case_preserve = %d, short case preserve = %d, " - "directory = %s, newname = %s, " - "last_component_dest = %s\n", - conn->case_sensitive, conn->case_preserve, - conn->short_case_preserve, - smb_fname_str_dbg(smb_fname_src), - smb_fname_str_dbg(smb_fname_dst), - dst_original_lcomp)); + ZERO_STRUCT(smb_fname_src->st); - ZERO_STRUCT(smb_fname_src->st); + rc = vfs_stat(conn, smb_fname_src); + if (rc == -1) { + status = map_nt_error_from_unix_common(errno); + goto out; + } - rc = vfs_stat(conn, smb_fname_src); - if (rc == -1) { - status = map_nt_error_from_unix_common(errno); + status = openat_pathref_fsp(conn->cwd_fsp, smb_fname_src); + if (!NT_STATUS_IS_OK(status)) { + if (!NT_STATUS_EQUAL(status, + NT_STATUS_OBJECT_NAME_NOT_FOUND)) { goto out; } - - status = openat_pathref_fsp(conn->cwd_fsp, smb_fname_src); - if (!NT_STATUS_IS_OK(status)) { - if (!NT_STATUS_EQUAL(status, - NT_STATUS_OBJECT_NAME_NOT_FOUND)) { - goto out; - } - /* - * Possible symlink src. - */ - if (!(smb_fname_src->flags & SMB_FILENAME_POSIX_PATH)) { - goto out; - } - if (!S_ISLNK(smb_fname_src->st.st_ex_mode)) { - goto out; - } + /* + * Possible symlink src. + */ + if (!(smb_fname_src->flags & SMB_FILENAME_POSIX_PATH)) { + goto out; } - - if (S_ISDIR(smb_fname_src->st.st_ex_mode)) { - create_options |= FILE_DIRECTORY_FILE; + if (!S_ISLNK(smb_fname_src->st.st_ex_mode)) { + goto out; } + } - status = SMB_VFS_CREATE_FILE( + if (S_ISDIR(smb_fname_src->st.st_ex_mode)) { + create_options |= FILE_DIRECTORY_FILE; + } + + status = SMB_VFS_CREATE_FILE( conn, /* conn */ req, /* req */ smb_fname_src, /* fname */ @@ -7839,27 +7837,25 @@ NTSTATUS rename_internals(TALLOC_CTX *ctx, posx, /* in_context_blobs */ NULL); /* out_context_blobs */ - if (!NT_STATUS_IS_OK(status)) { - DEBUG(3, ("Could not open rename source %s: %s\n", - smb_fname_str_dbg(smb_fname_src), - nt_errstr(status))); - goto out; - } + if (!NT_STATUS_IS_OK(status)) { + DBG_NOTICE("Could not open rename source %s: %s\n", + smb_fname_str_dbg(smb_fname_src), + nt_errstr(status)); + goto out; + } - status = rename_internals_fsp(conn, + status = rename_internals_fsp(conn, fsp, smb_fname_dst, dst_original_lcomp, attrs, replace_if_exists); - close_file(req, fsp, NORMAL_CLOSE); + close_file(req, fsp, NORMAL_CLOSE); - DEBUG(3, ("rename_internals: Error %s rename %s -> %s\n", - nt_errstr(status), smb_fname_str_dbg(smb_fname_src), - smb_fname_str_dbg(smb_fname_dst))); - - } + DBG_NOTICE("Error %s rename %s -> %s\n", + nt_errstr(status), smb_fname_str_dbg(smb_fname_src), + smb_fname_str_dbg(smb_fname_dst)); out: TALLOC_FREE(posx); -- 2.32.0 From 4706138a3539d70e5d2de3af5f57f717aa5bbe34 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 1 Dec 2021 16:39:42 -0800 Subject: [PATCH 37/92] CVE-2021-44141: s3: smbd: Remove 'const char *src_original_lcomp' parameter from rename_internals(). No longer used. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source3/smbd/nttrans.c | 1 - source3/smbd/proto.h | 1 - source3/smbd/reply.c | 2 -- source3/smbd/trans2.c | 1 - 4 files changed, 5 deletions(-) diff --git a/source3/smbd/nttrans.c b/source3/smbd/nttrans.c index b0f55ade267..2b742411071 100644 --- a/source3/smbd/nttrans.c +++ b/source3/smbd/nttrans.c @@ -1853,7 +1853,6 @@ void reply_ntrename(struct smb_request *req) conn, req, smb_fname_old, - NULL, smb_fname_new, dst_original_lcomp, attrs, diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h index 9454d44968b..fa26d58bfde 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -1041,7 +1041,6 @@ NTSTATUS rename_internals(TALLOC_CTX *ctx, connection_struct *conn, struct smb_request *req, struct smb_filename *smb_fname_src, - const char *src_original_lcomp, struct smb_filename *smb_fname_dst, const char *dst_original_lcomp, uint32_t attrs, diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index e9814037b1e..8fc943375b4 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -7694,7 +7694,6 @@ NTSTATUS rename_internals(TALLOC_CTX *ctx, connection_struct *conn, struct smb_request *req, struct smb_filename *smb_fname_src, - const char *src_original_lcomp, struct smb_filename *smb_fname_dst, const char *dst_original_lcomp, uint32_t attrs, @@ -7996,7 +7995,6 @@ void reply_mv(struct smb_request *req) conn, req, smb_fname_src, - src_original_lcomp, smb_fname_dst, dst_original_lcomp, attrs, diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c index f844ed8de78..a8ef015b5cd 100644 --- a/source3/smbd/trans2.c +++ b/source3/smbd/trans2.c @@ -7525,7 +7525,6 @@ static NTSTATUS smb_file_rename_information(connection_struct *conn, conn, req, smb_fname_src, - NULL, smb_fname_dst, dst_original_lcomp, 0, -- 2.32.0 From bfc91e154835950a796b3307f87f1e92c963986e Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 1 Dec 2021 16:40:55 -0800 Subject: [PATCH 38/92] CVE-2021-44141: s3: smbd: Remove 'const char *src_original_lcomp' from reply_mv(). No longer used. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme --- source3/smbd/reply.c | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index 8fc943375b4..079c8da7e3c 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -7879,7 +7879,6 @@ void reply_mv(struct smb_request *req) NTSTATUS status; TALLOC_CTX *ctx = talloc_tos(); struct smb_filename *smb_fname_src = NULL; - const char *src_original_lcomp = NULL; struct smb_filename *smb_fname_dst = NULL; const char *dst_original_lcomp = NULL; uint32_t src_ucf_flags = ucf_flags_from_smb_request(req); @@ -7939,16 +7938,6 @@ void reply_mv(struct smb_request *req) goto out; } - /* Get the last component of the source for rename_internals(). */ - src_original_lcomp = get_original_lcomp(ctx, - conn, - name, - dst_ucf_flags); - if (src_original_lcomp == NULL) { - reply_nterror(req, NT_STATUS_NO_MEMORY); - goto out; - } - status = filename_convert(ctx, conn, newname, -- 2.32.0 From 366a5d04b22c05ae0eda786205d415821a97ade6 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 9 Dec 2021 16:08:07 -0800 Subject: [PATCH 39/92] CVE-2021-44141: s3: smbd: Move setting of dirtype if FILE_ATTRIBUTE_NORMAL to do_unlink(). Now we don't use wildcards when calling in unlink_internals() the logic inside it serves no purpose and can be replaced with a direct call to do_unlink() (which we will rename to unlink_internals()). BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/reply.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index 079c8da7e3c..b709dc65c47 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -3142,6 +3142,10 @@ static NTSTATUS do_unlink(connection_struct *conn, int ret; struct smb2_create_blobs *posx = NULL; + if (dirtype == 0) { + dirtype = FILE_ATTRIBUTE_NORMAL; + } + DEBUG(10,("do_unlink: %s, dirtype = %d\n", smb_fname_str_dbg(smb_fname), dirtype)); @@ -3333,9 +3337,6 @@ NTSTATUS unlink_internals(connection_struct *conn, status = NT_STATUS_NO_MEMORY; goto out; } - if (dirtype == 0) { - dirtype = FILE_ATTRIBUTE_NORMAL; - } status = check_name(conn, smb_fname); if (!NT_STATUS_IS_OK(status)) { -- 2.32.0 From bd825c3e7ff5f271a2997a8db19c746ed2288047 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 9 Dec 2021 16:11:20 -0800 Subject: [PATCH 40/92] CVE-2021-44141: s3: smbd: Move to modern debug calls inside do_unlink(). We will be changing its name next. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/reply.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index b709dc65c47..71d7454e58f 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -3146,9 +3146,9 @@ static NTSTATUS do_unlink(connection_struct *conn, dirtype = FILE_ATTRIBUTE_NORMAL; } - DEBUG(10,("do_unlink: %s, dirtype = %d\n", + DBG_DEBUG("%s, dirtype = %d\n", smb_fname_str_dbg(smb_fname), - dirtype)); + dirtype); if (!CAN_WRITE(conn)) { return NT_STATUS_MEDIA_WRITE_PROTECTED; @@ -3248,17 +3248,17 @@ static NTSTATUS do_unlink(connection_struct *conn, TALLOC_FREE(posx); if (!NT_STATUS_IS_OK(status)) { - DEBUG(10, ("SMB_VFS_CREATEFILE failed: %s\n", - nt_errstr(status))); + DBG_DEBUG("SMB_VFS_CREATEFILE failed: %s\n", + nt_errstr(status)); return status; } status = can_set_delete_on_close(fsp, fattr); if (!NT_STATUS_IS_OK(status)) { - DEBUG(10, ("do_unlink can_set_delete_on_close for file %s - " + DBG_DEBUG("can_set_delete_on_close for file %s - " "(%s)\n", smb_fname_str_dbg(smb_fname), - nt_errstr(status))); + nt_errstr(status)); close_file(req, fsp, NORMAL_CLOSE); return status; } -- 2.32.0 From 2d9a08a2d91ef0cd710801499537b8a4418ed4e5 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 9 Dec 2021 16:14:40 -0800 Subject: [PATCH 41/92] CVE-2021-44141: s3: smbd: Comment out the old unlink_internals(). Rename do_unlink() -> unlink_internals(). One parameter needs changing position. The logic inside unlink_internals() is no longer needed if it doesn't accept wildcards. filename_convert() already handles mangled names just fine, so we don't need this logic. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/reply.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index 71d7454e58f..01de8f0c03f 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -3130,10 +3130,10 @@ static NTSTATUS can_rename(connection_struct *conn, files_struct *fsp, * unlink a file with all relevant access checks *******************************************************************/ -static NTSTATUS do_unlink(connection_struct *conn, +NTSTATUS unlink_internals(connection_struct *conn, struct smb_request *req, - struct smb_filename *smb_fname, - uint32_t dirtype) + uint32_t dirtype, + struct smb_filename *smb_fname) { uint32_t fattr; files_struct *fsp; @@ -3274,6 +3274,7 @@ static NTSTATUS do_unlink(connection_struct *conn, return close_file(req, fsp, NORMAL_CLOSE); } +#if 0 /**************************************************************************** The guts of the unlink command, split out so it may be called by the NT SMB code. @@ -3351,6 +3352,7 @@ NTSTATUS unlink_internals(connection_struct *conn, TALLOC_FREE(fname_mask); return status; } +#endif /**************************************************************************** Reply to a unlink -- 2.32.0 From 9b9b9d9ef4c3eb4a03dea3cff446da42738f01c9 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 9 Dec 2021 16:16:52 -0800 Subject: [PATCH 42/92] CVE-2021-44141: s3: smbd: Remove the old unlink_internals() implementation. No longer used. filename_convert() already handles mangled names just fine, so we don't need this logic. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/reply.c | 80 -------------------------------------------- 1 file changed, 80 deletions(-) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index 01de8f0c03f..26049b2b62b 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -3274,86 +3274,6 @@ NTSTATUS unlink_internals(connection_struct *conn, return close_file(req, fsp, NORMAL_CLOSE); } -#if 0 -/**************************************************************************** - The guts of the unlink command, split out so it may be called by the NT SMB - code. -****************************************************************************/ - -NTSTATUS unlink_internals(connection_struct *conn, - struct smb_request *req, - uint32_t dirtype, - struct smb_filename *smb_fname) -{ - char *fname_dir = NULL; - char *fname_mask = NULL; - NTSTATUS status = NT_STATUS_OK; - struct smb_filename *smb_fname_dir = NULL; - TALLOC_CTX *ctx = talloc_tos(); - - /* Split up the directory from the filename/mask. */ - status = split_fname_dir_mask(ctx, smb_fname->base_name, - &fname_dir, &fname_mask); - if (!NT_STATUS_IS_OK(status)) { - goto out; - } - - /* - * We should only check the mangled cache - * here if unix_convert failed. This means - * that the path in 'mask' doesn't exist - * on the file system and so we need to look - * for a possible mangle. This patch from - * Tine Smukavec . - */ - - if (!VALID_STAT(smb_fname->st) && - mangle_is_mangled(fname_mask, conn->params)) { - char *new_mask = NULL; - mangle_lookup_name_from_8_3(ctx, fname_mask, - &new_mask, conn->params); - if (new_mask) { - TALLOC_FREE(fname_mask); - fname_mask = new_mask; - } - } - - /* - * Only one file needs to be unlinked. Append the mask back - * onto the directory. - */ - TALLOC_FREE(smb_fname->base_name); - if (ISDOT(fname_dir)) { - /* Ensure we use canonical names on open. */ - smb_fname->base_name = talloc_asprintf(smb_fname, - "%s", - fname_mask); - } else { - smb_fname->base_name = talloc_asprintf(smb_fname, - "%s/%s", - fname_dir, - fname_mask); - } - if (!smb_fname->base_name) { - status = NT_STATUS_NO_MEMORY; - goto out; - } - - status = check_name(conn, smb_fname); - if (!NT_STATUS_IS_OK(status)) { - goto out; - } - - status = do_unlink(conn, req, smb_fname, dirtype); - - out: - TALLOC_FREE(smb_fname_dir); - TALLOC_FREE(fname_dir); - TALLOC_FREE(fname_mask); - return status; -} -#endif - /**************************************************************************** Reply to a unlink ****************************************************************************/ -- 2.32.0 From a902664eeda10f7db4f63aa00ab64bfd273e2f28 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 9 Dec 2021 16:35:17 -0800 Subject: [PATCH 43/92] CVE-2021-44141: s3: smbd: Handling SMB_FILE_RENAME_INFORMATION, the destination name is a single component. No errors should be allowed from filename_convert(). BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/trans2.c | 19 +------------------ 1 file changed, 1 insertion(+), 18 deletions(-) diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c index a8ef015b5cd..feccc4cee45 100644 --- a/source3/smbd/trans2.c +++ b/source3/smbd/trans2.c @@ -7475,25 +7475,8 @@ static NTSTATUS smb_file_rename_information(connection_struct *conn, 0, &smb_fname_dst); - /* If an error we expect this to be - * NT_STATUS_OBJECT_PATH_NOT_FOUND */ - if (!NT_STATUS_IS_OK(status)) { - if(!NT_STATUS_EQUAL(NT_STATUS_OBJECT_PATH_NOT_FOUND, - status)) { - goto out; - } - /* Create an smb_fname to call rename_internals_fsp() */ - smb_fname_dst = synthetic_smb_fname(ctx, - base_name, - NULL, - NULL, - smb_fname_src->twrp, - smb_fname_src->flags); - if (smb_fname_dst == NULL) { - status = NT_STATUS_NO_MEMORY; - goto out; - } + goto out; } dst_original_lcomp = get_original_lcomp(smb_fname_dst, conn, -- 2.32.0 From 0e074a6a6680e11f48d7dc7b4fd4bb6b91f80b81 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 9 Dec 2021 16:45:13 -0800 Subject: [PATCH 44/92] CVE-2021-44141: s3: smbd: In rename_internals_fsp(), remove unneeded call to check_name(). All callers have gone through filename_convert(), which has already called check_name() on the destination. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/reply.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index 26049b2b62b..b9f1bd6b9c2 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -7185,11 +7185,6 @@ NTSTATUS rename_internals_fsp(connection_struct *conn, bool dst_exists, old_is_stream, new_is_stream; int ret; - status = check_name(conn, smb_fname_dst_in); - if (!NT_STATUS_IS_OK(status)) { - return status; - } - status = parent_dirname_compatible_open(conn, smb_fname_dst_in); if (!NT_STATUS_IS_OK(status)) { return status; -- 2.32.0 From 430ca31b2363ff99fc4d501b471602267bab44b6 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 9 Dec 2021 16:47:13 -0800 Subject: [PATCH 45/92] CVE-2021-44141: s3: smbd: check_name() is now static to filename.c BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/filename.c | 2 +- source3/smbd/proto.h | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index 11022dca703..44f9aebf068 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -1430,7 +1430,7 @@ static NTSTATUS check_veto_path(connection_struct *conn, a valid one for the user to access. ****************************************************************************/ -NTSTATUS check_name(connection_struct *conn, +static NTSTATUS check_name(connection_struct *conn, const struct smb_filename *smb_fname) { NTSTATUS status = check_veto_path(conn, smb_fname); diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h index fa26d58bfde..e6a0d6c3d51 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -358,8 +358,6 @@ NTSTATUS unix_convert(TALLOC_CTX *ctx, NTTIME twrp, struct smb_filename **smb_fname, uint32_t ucf_flags); -NTSTATUS check_name(connection_struct *conn, - const struct smb_filename *smb_fname); NTSTATUS canonicalize_snapshot_path(struct smb_filename *smb_fname, uint32_t ucf_flags, NTTIME twrp); -- 2.32.0 From d1fedab44898ec8fc39a0ca29455e9da6e75f3b2 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 9 Dec 2021 16:49:46 -0800 Subject: [PATCH 46/92] CVE-2021-44141: s3: smbd: In rename_internals(), remove the name spliting and re-combining code. filename_convert() handles mangled names just fine, so we don't need to split the last component and check for mangle. Now we don't take wildcard names this is not needed. This was the last caller of split_fname_dir_mask(), so ifdef it out. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/reply.c | 67 ++------------------------------------------ 1 file changed, 2 insertions(+), 65 deletions(-) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index b9f1bd6b9c2..7b0eb18d744 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -1624,6 +1624,7 @@ void reply_dskattr(struct smb_request *req) return; } +#if 0 /* * Utility function to split the filename from the directory. */ @@ -1655,6 +1656,7 @@ static NTSTATUS split_fname_dir_mask(TALLOC_CTX *ctx, const char *fname_in, *fname_mask_out = fname_mask; return NT_STATUS_OK; } +#endif /**************************************************************************** Make a dir struct. @@ -7618,52 +7620,12 @@ NTSTATUS rename_internals(TALLOC_CTX *ctx, bool replace_if_exists, uint32_t access_mask) { - char *fname_src_dir = NULL; - struct smb_filename *smb_fname_src_dir = NULL; - char *fname_src_mask = NULL; NTSTATUS status = NT_STATUS_OK; - char *talloced = NULL; int create_options = 0; struct smb2_create_blobs *posx = NULL; struct files_struct *fsp = NULL; int rc; - /* - * Split the old name into directory and last component - * strings. Note that unix_convert may have stripped off a - * leading ./ from both name and newname if the rename is - * at the root of the share. We need to make sure either both - * name and newname contain a / character or neither of them do. - */ - - /* Split up the directory from the filename/mask. */ - status = split_fname_dir_mask(ctx, smb_fname_src->base_name, - &fname_src_dir, &fname_src_mask); - if (!NT_STATUS_IS_OK(status)) { - status = NT_STATUS_NO_MEMORY; - goto out; - } - - /* - * We should only check the mangled cache - * here if unix_convert failed. This means - * that the path in 'mask' doesn't exist - * on the file system and so we need to look - * for a possible mangle. This patch from - * Tine Smukavec . - */ - - if (!VALID_STAT(smb_fname_src->st) && - mangle_is_mangled(fname_src_mask, conn->params)) { - char *new_mask = NULL; - mangle_lookup_name_from_8_3(ctx, fname_src_mask, &new_mask, - conn->params); - if (new_mask) { - TALLOC_FREE(fname_src_mask); - fname_src_mask = new_mask; - } - } - if (smb_fname_src->flags & SMB_FILENAME_POSIX_PATH) { status = make_smb2_posix_create_ctx(talloc_tos(), &posx, 0777); if (!NT_STATUS_IS_OK(status)) { @@ -7673,27 +7635,6 @@ NTSTATUS rename_internals(TALLOC_CTX *ctx, } } - /* - * Only one file needs to be renamed. Append the mask back - * onto the directory. - */ - TALLOC_FREE(smb_fname_src->base_name); - if (ISDOT(fname_src_dir)) { - /* Ensure we use canonical names on open. */ - smb_fname_src->base_name = talloc_asprintf(smb_fname_src, - "%s", - fname_src_mask); - } else { - smb_fname_src->base_name = talloc_asprintf(smb_fname_src, - "%s/%s", - fname_src_dir, - fname_src_mask); - } - if (!smb_fname_src->base_name) { - status = NT_STATUS_NO_MEMORY; - goto out; - } - DBG_NOTICE("case_sensitive = %d, " "case_preserve = %d, short case preserve = %d, " "directory = %s, newname = %s, " @@ -7776,10 +7717,6 @@ NTSTATUS rename_internals(TALLOC_CTX *ctx, out: TALLOC_FREE(posx); - TALLOC_FREE(talloced); - TALLOC_FREE(smb_fname_src_dir); - TALLOC_FREE(fname_src_dir); - TALLOC_FREE(fname_src_mask); return status; } -- 2.32.0 From 1d6ebbecb3c6180cf544c0ba99948ebe5ebe16e7 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 9 Dec 2021 16:51:45 -0800 Subject: [PATCH 47/92] CVE-2021-44141: s3: smbd: Remove split_fname_dir_mask(). No longer used. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/reply.c | 34 ---------------------------------- 1 file changed, 34 deletions(-) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index 7b0eb18d744..d2048856cff 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -1624,40 +1624,6 @@ void reply_dskattr(struct smb_request *req) return; } -#if 0 -/* - * Utility function to split the filename from the directory. - */ -static NTSTATUS split_fname_dir_mask(TALLOC_CTX *ctx, const char *fname_in, - char **fname_dir_out, - char **fname_mask_out) -{ - const char *p = NULL; - char *fname_dir = NULL; - char *fname_mask = NULL; - - p = strrchr_m(fname_in, '/'); - if (!p) { - fname_dir = talloc_strdup(ctx, "."); - fname_mask = talloc_strdup(ctx, fname_in); - } else { - fname_dir = talloc_strndup(ctx, fname_in, - PTR_DIFF(p, fname_in)); - fname_mask = talloc_strdup(ctx, p+1); - } - - if (!fname_dir || !fname_mask) { - TALLOC_FREE(fname_dir); - TALLOC_FREE(fname_mask); - return NT_STATUS_NO_MEMORY; - } - - *fname_dir_out = fname_dir; - *fname_mask_out = fname_mask; - return NT_STATUS_OK; -} -#endif - /**************************************************************************** Make a dir struct. ****************************************************************************/ -- 2.32.0 From 92523a9f5b45e79c14138f5b6b19a2fe390c83a8 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 2 Dec 2021 17:51:42 -0800 Subject: [PATCH 48/92] CVE-2021-44141: s3: smbd: In call_trans2findfirst() we don't need filename_convert_with_privilege() anymore. It was extra-paranoid code now not needed as the new VFS version of filename_convert() does the same job. There are now no remaining callers of filename_convert_with_privilege(). BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/trans2.c | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c index feccc4cee45..06077a87dad 100644 --- a/source3/smbd/trans2.c +++ b/source3/smbd/trans2.c @@ -2755,19 +2755,12 @@ close_if_end = %d requires_resume_key = %d backup_priv = %d level = 0x%x, max_da if (backup_priv) { become_root(); as_root = true; - ntstatus = filename_convert_with_privilege(talloc_tos(), - conn, - req, - directory, - ucf_flags, - &smb_dname); - } else { - ntstatus = filename_convert(talloc_tos(), conn, + } + ntstatus = filename_convert(talloc_tos(), conn, directory, ucf_flags, 0, &smb_dname); - } if (!NT_STATUS_IS_OK(ntstatus)) { if (NT_STATUS_EQUAL(ntstatus,NT_STATUS_PATH_NOT_COVERED)) { -- 2.32.0 From 29b0a142a381fece79e051ecde3d89ba9be4ef22 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 2 Dec 2021 17:55:26 -0800 Subject: [PATCH 49/92] CVE-2021-44141: s3: smbd: Remove filename_convert_with_privilege(). No longer used. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/filename.c | 21 --------------------- source3/smbd/proto.h | 6 ------ 2 files changed, 27 deletions(-) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index 44f9aebf068..3d0f350c3dc 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -2078,27 +2078,6 @@ NTSTATUS filename_convert(TALLOC_CTX *ctx, pp_smb_fname); } -/* - * Go through all the steps to validate a filename. - * root (privileged) version. - */ - -NTSTATUS filename_convert_with_privilege(TALLOC_CTX *ctx, - connection_struct *conn, - struct smb_request *smbreq, - const char *name_in, - uint32_t ucf_flags, - struct smb_filename **pp_smb_fname) -{ - return filename_convert_internal(ctx, - conn, - smbreq, - name_in, - ucf_flags, - 0, - pp_smb_fname); -} - /* * Build the full path from a dirfsp and dirfsp relative name */ diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h index e6a0d6c3d51..58b32c6908b 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -382,12 +382,6 @@ NTSTATUS filename_convert(TALLOC_CTX *mem_ctx, uint32_t ucf_flags, NTTIME twrp, struct smb_filename **pp_smb_fname); -NTSTATUS filename_convert_with_privilege(TALLOC_CTX *mem_ctx, - connection_struct *conn, - struct smb_request *smbreq, - const char *name_in, - uint32_t ucf_flags, - struct smb_filename **pp_smb_fname); /* The following definitions come from smbd/files.c */ -- 2.32.0 From 23527cb4e74e1ed1be486e2f1fd670061f429855 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 10:10:45 -0800 Subject: [PATCH 50/92] CVE-2021-44141: s3: smbd: In filename_convert_internal(), remove call to check_name_with_privilege(). We now always pass NULL as struct smb_request *smbreq, so this code path can never be taken. Comment out check_name_with_privilege() as it's now no longer used. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/filename.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index 3d0f350c3dc..882f41ec400 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -1452,6 +1452,7 @@ static NTSTATUS check_name(connection_struct *conn, return NT_STATUS_OK; } +#if 0 /**************************************************************************** Must be called as root. Creates the struct privilege_paths attached to the struct smb_request if this call is successful. @@ -1470,6 +1471,7 @@ static NTSTATUS check_name_with_privilege(connection_struct *conn, smb_fname, smbreq); } +#endif /**************************************************************************** Check if two filenames are equal. @@ -1999,11 +2001,8 @@ static NTSTATUS filename_convert_internal(TALLOC_CTX *ctx, TALLOC_FREE(smb_fname); return status; } - } else if (!smbreq) { - status = check_name(conn, smb_fname); } else { - status = check_name_with_privilege(conn, smbreq, - smb_fname); + status = check_name(conn, smb_fname); } if (!NT_STATUS_IS_OK(status)) { DEBUG(3,("filename_convert_internal: check_name failed " -- 2.32.0 From 0b5a5d577d2f54a25d8ca50b20062bd605549c76 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 10:13:13 -0800 Subject: [PATCH 51/92] CVE-2021-44141: s3: smbd: Remove unused check_name_with_privilege(). BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/filename.c | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index 882f41ec400..297f48496e8 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -1452,27 +1452,6 @@ static NTSTATUS check_name(connection_struct *conn, return NT_STATUS_OK; } -#if 0 -/**************************************************************************** - Must be called as root. Creates the struct privilege_paths - attached to the struct smb_request if this call is successful. -****************************************************************************/ - -static NTSTATUS check_name_with_privilege(connection_struct *conn, - struct smb_request *smbreq, - const struct smb_filename *smb_fname) -{ - NTSTATUS status = check_veto_path(conn, smb_fname); - - if (!NT_STATUS_IS_OK(status)) { - return status; - } - return check_reduced_name_with_privilege(conn, - smb_fname, - smbreq); -} -#endif - /**************************************************************************** Check if two filenames are equal. This needs to be careful about whether we are case sensitive. -- 2.32.0 From 57c6c5967e161880591cf8bfcb7313ab4447f57e Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 10:14:03 -0800 Subject: [PATCH 52/92] CVE-2021-44141: s3: smbd: Remove now unused check_reduced_name_with_privilege(). We now only have one function that does this check (check_reduced_name()), used everywhere. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/proto.h | 3 - source3/smbd/vfs.c | 173 ------------------------------------------- 2 files changed, 176 deletions(-) diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h index 58b32c6908b..5e7bc392b87 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -1300,9 +1300,6 @@ struct smb_filename *vfs_GetWd(TALLOC_CTX *ctx, connection_struct *conn); NTSTATUS check_reduced_name(connection_struct *conn, const struct smb_filename *cwd_fname, const struct smb_filename *smb_fname); -NTSTATUS check_reduced_name_with_privilege(connection_struct *conn, - const struct smb_filename *smb_fname, - struct smb_request *smbreq); int vfs_stat(struct connection_struct *conn, struct smb_filename *smb_fname); int vfs_stat_smb_basename(struct connection_struct *conn, diff --git a/source3/smbd/vfs.c b/source3/smbd/vfs.c index 34831385e09..d8b7b1283fb 100644 --- a/source3/smbd/vfs.c +++ b/source3/smbd/vfs.c @@ -1122,179 +1122,6 @@ struct smb_filename *vfs_GetWd(TALLOC_CTX *ctx, connection_struct *conn) return result; } -/******************************************************************* - Reduce a file name, removing .. elements and checking that - it is below dir in the hierarchy. This uses realpath. - This function must run as root, and will return names - and valid stat structs that can be checked on open. -********************************************************************/ - -NTSTATUS check_reduced_name_with_privilege(connection_struct *conn, - const struct smb_filename *smb_fname, - struct smb_request *smbreq) -{ - NTSTATUS status; - TALLOC_CTX *ctx = talloc_tos(); - const char *conn_rootdir; - size_t rootdir_len; - char *resolved_name = NULL; - struct smb_filename *resolved_fname = NULL; - struct smb_filename *saved_dir_fname = NULL; - struct smb_filename *smb_fname_cwd = NULL; - int ret; - struct smb_filename *parent_name = NULL; - struct smb_filename *file_name = NULL; - - DEBUG(3,("check_reduced_name_with_privilege [%s] [%s]\n", - smb_fname->base_name, - conn->connectpath)); - - status = SMB_VFS_PARENT_PATHNAME(conn, - ctx, - smb_fname, - &parent_name, - &file_name); - if (!NT_STATUS_IS_OK(status)) { - goto err; - } - - if (SMB_VFS_STAT(conn, parent_name) != 0) { - status = map_nt_error_from_unix(errno); - goto err; - } - /* Remember where we were. */ - saved_dir_fname = vfs_GetWd(ctx, conn); - if (!saved_dir_fname) { - status = map_nt_error_from_unix(errno); - goto err; - } - - if (vfs_ChDir(conn, parent_name) == -1) { - status = map_nt_error_from_unix(errno); - goto err; - } - - smb_fname_cwd = synthetic_smb_fname(talloc_tos(), - ".", - NULL, - NULL, - parent_name->twrp, - 0); - if (smb_fname_cwd == NULL) { - status = NT_STATUS_NO_MEMORY; - goto err; - } - - /* Get the absolute path of the parent directory. */ - resolved_fname = SMB_VFS_REALPATH(conn, ctx, smb_fname_cwd); - if (resolved_fname == NULL) { - status = map_nt_error_from_unix(errno); - goto err; - } - resolved_name = resolved_fname->base_name; - - if (*resolved_name != '/') { - DEBUG(0,("check_reduced_name_with_privilege: realpath " - "doesn't return absolute paths !\n")); - status = NT_STATUS_OBJECT_NAME_INVALID; - goto err; - } - - DBG_DEBUG("realpath [%s] -> [%s]\n", - smb_fname_str_dbg(parent_name), - resolved_name); - - /* Now check the stat value is the same. */ - if (SMB_VFS_LSTAT(conn, smb_fname_cwd) != 0) { - status = map_nt_error_from_unix(errno); - goto err; - } - - /* Ensure we're pointing at the same place. */ - if (!check_same_stat(&smb_fname_cwd->st, &parent_name->st)) { - DBG_ERR("device/inode/uid/gid on directory %s changed. " - "Denying access !\n", - smb_fname_str_dbg(parent_name)); - status = NT_STATUS_ACCESS_DENIED; - goto err; - } - - /* Ensure we're below the connect path. */ - - conn_rootdir = SMB_VFS_CONNECTPATH(conn, smb_fname); - if (conn_rootdir == NULL) { - DEBUG(2, ("check_reduced_name_with_privilege: Could not get " - "conn_rootdir\n")); - status = NT_STATUS_ACCESS_DENIED; - goto err; - } - - rootdir_len = strlen(conn_rootdir); - - /* - * In the case of rootdir_len == 1, we know that conn_rootdir is - * "/", and we also know that resolved_name starts with a slash. - * So, in this corner case, resolved_name is automatically a - * sub-directory of the conn_rootdir. Thus we can skip the string - * comparison and the next character checks (which are even - * wrong in this case). - */ - if (rootdir_len != 1) { - bool matched; - - matched = (strncmp(conn_rootdir, resolved_name, - rootdir_len) == 0); - - if (!matched || (resolved_name[rootdir_len] != '/' && - resolved_name[rootdir_len] != '\0')) { - DBG_WARNING("%s is a symlink outside the " - "share path\n", - smb_fname_str_dbg(parent_name)); - DEBUGADD(1, ("conn_rootdir =%s\n", conn_rootdir)); - DEBUGADD(1, ("resolved_name=%s\n", resolved_name)); - status = NT_STATUS_ACCESS_DENIED; - goto err; - } - } - - /* Now ensure that the last component either doesn't - exist, or is *NOT* a symlink. */ - - ret = SMB_VFS_LSTAT(conn, file_name); - if (ret == -1) { - /* Errno must be ENOENT for this be ok. */ - if (errno != ENOENT) { - status = map_nt_error_from_unix(errno); - DBG_WARNING("LSTAT on %s failed with %s\n", - smb_fname_str_dbg(file_name), - nt_errstr(status)); - goto err; - } - } - - if (VALID_STAT(file_name->st) && - S_ISLNK(file_name->st.st_ex_mode)) - { - DBG_WARNING("Last component %s is a symlink. Denying" - "access.\n", - smb_fname_str_dbg(file_name)); - status = NT_STATUS_ACCESS_DENIED; - goto err; - } - - status = NT_STATUS_OK; - - err: - - if (saved_dir_fname != NULL) { - vfs_ChDir(conn, saved_dir_fname); - TALLOC_FREE(saved_dir_fname); - } - TALLOC_FREE(resolved_fname); - TALLOC_FREE(parent_name); - return status; -} - /******************************************************************* Reduce a file name, removing .. elements and checking that it is below dir in the hierarchy. This uses realpath. -- 2.32.0 From b446d6f40284ad14c1f792431b5ef6a5ec8400cb Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 10:19:38 -0800 Subject: [PATCH 53/92] CVE-2021-44141: s3: smbd: filename_convert() is now a one-to-one wrapper around filename_convert_internal(). Remove filename_convert() and rename filename_convert_internal() -> filename_convert(). Move the old DEBUG(..) statements to DBG_XXX() so they don't print the wrong name. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/filename.c | 46 +++++++++++------------------------------ 1 file changed, 12 insertions(+), 34 deletions(-) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index 297f48496e8..2c4fa5506ff 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -1913,13 +1913,12 @@ char *get_original_lcomp(TALLOC_CTX *ctx, * @return NT_STATUS_OK if all operations completed successfully, appropriate * error otherwise. */ -static NTSTATUS filename_convert_internal(TALLOC_CTX *ctx, - connection_struct *conn, - struct smb_request *smbreq, - const char *name_in, - uint32_t ucf_flags, - NTTIME twrp, - struct smb_filename **_smb_fname) +NTSTATUS filename_convert(TALLOC_CTX *ctx, + connection_struct *conn, + const char *name_in, + uint32_t ucf_flags, + NTTIME twrp, + struct smb_filename **_smb_fname) { struct smb_filename *smb_fname = NULL; bool has_wild; @@ -1935,10 +1934,10 @@ static NTSTATUS filename_convert_internal(TALLOC_CTX *ctx, !conn->sconn->using_smb2, &fname); if (!NT_STATUS_IS_OK(status)) { - DEBUG(10,("filename_convert_internal: dfs_redirect " + DBG_DEBUG("dfs_redirect " "failed for name %s with %s\n", name_in, - nt_errstr(status) )); + nt_errstr(status)); return status; } name_in = fname; @@ -1964,10 +1963,10 @@ static NTSTATUS filename_convert_internal(TALLOC_CTX *ctx, status = unix_convert(ctx, conn, name_in, twrp, &smb_fname, ucf_flags); if (!NT_STATUS_IS_OK(status)) { - DEBUG(10,("filename_convert_internal: unix_convert failed " + DBG_DEBUG("unix_convert failed " "for name %s with %s\n", name_in, - nt_errstr(status) )); + nt_errstr(status)); return status; } @@ -1984,10 +1983,10 @@ static NTSTATUS filename_convert_internal(TALLOC_CTX *ctx, status = check_name(conn, smb_fname); } if (!NT_STATUS_IS_OK(status)) { - DEBUG(3,("filename_convert_internal: check_name failed " + DBG_NOTICE("check_name failed " "for name %s with %s\n", smb_fname_str_dbg(smb_fname), - nt_errstr(status) )); + nt_errstr(status)); TALLOC_FREE(smb_fname); return status; } @@ -2035,27 +2034,6 @@ static NTSTATUS filename_convert_internal(TALLOC_CTX *ctx, return status; } -/* - * Go through all the steps to validate a filename. - * Non-root version. - */ - -NTSTATUS filename_convert(TALLOC_CTX *ctx, - connection_struct *conn, - const char *name_in, - uint32_t ucf_flags, - NTTIME twrp, - struct smb_filename **pp_smb_fname) -{ - return filename_convert_internal(ctx, - conn, - NULL, - name_in, - ucf_flags, - twrp, - pp_smb_fname); -} - /* * Build the full path from a dirfsp and dirfsp relative name */ -- 2.32.0 From 108a27938cf2f5e6fae539175b9cae9705e08466 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 16:00:26 -0800 Subject: [PATCH 54/92] CVE-2021-44141: s3: smbd: In dfs_path_lookup(). If we have a DFS path including a @GMT-token, don't throw away the twrp value when parsing the path. Not yet used. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/msdfs.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/source3/smbd/msdfs.c b/source3/smbd/msdfs.c index fd002e98071..86ca79a994b 100644 --- a/source3/smbd/msdfs.c +++ b/source3/smbd/msdfs.c @@ -697,6 +697,7 @@ static NTSTATUS dfs_path_lookup(TALLOC_CTX *ctx, const struct dfs_path *pdp, /* Parsed out server+share+extrapath. */ uint32_t ucf_flags, + NTTIME *_twrp, int *consumedcntp, struct referral **ppreflist, size_t *preferral_count) @@ -867,6 +868,10 @@ static NTSTATUS dfs_path_lookup(TALLOC_CTX *ctx, } } + if ((ucf_flags & UCF_GMT_PATHNAME) && _twrp != NULL) { + *_twrp = smb_fname->twrp; + } + status = NT_STATUS_OK; out: @@ -965,6 +970,7 @@ NTSTATUS dfs_redirect(TALLOC_CTX *ctx, path_in, pdp, ucf_flags, + NULL, /* twrp. */ NULL, /* int *consumedcntp */ NULL, /* struct referral **ppreflist */ NULL); /* size_t *preferral_count */ @@ -1189,6 +1195,7 @@ NTSTATUS get_referred_path(TALLOC_CTX *ctx, dfs_path, pdp, 0, /* ucf_flags */ + NULL, consumedcntp, &jucn->referral_list, &jucn->referral_count); -- 2.32.0 From 6c448102d56b00d83e3e914e4b4116270aa183bf Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 16:14:08 -0800 Subject: [PATCH 55/92] CVE-2021-44141: s3: smbd: Allow dfs_redirect() to return a TWRP token it got from a parsed pathname. This one is subtle. If an SMB1 request has both a DFS path and a @GMT token, the unix_convert() inside the DFS path processing will remove the @GMT token, not allowing the subsequent unix_convert() inside filename_convert() to see it. By returning it from dfs_redirect() we can ensure it's correctly added to the smb_filename returned from filename_convert(). BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/filename.c | 11 +++++++++-- source3/smbd/msdfs.c | 3 ++- source3/smbd/proto.h | 1 + 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index 2c4fa5506ff..bcbe9f70015 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -1824,6 +1824,7 @@ char *get_original_lcomp(TALLOC_CTX *ctx, char *last_slash = NULL; char *orig_lcomp; char *fname = NULL; + NTTIME twrp = 0; NTSTATUS status; if (ucf_flags & UCF_DFS_PATHNAME) { @@ -1832,6 +1833,7 @@ char *get_original_lcomp(TALLOC_CTX *ctx, filename_in, ucf_flags, !conn->sconn->using_smb2, + &twrp, &fname); if (!NT_STATUS_IS_OK(status)) { DBG_DEBUG("dfs_redirect " @@ -1860,7 +1862,7 @@ char *get_original_lcomp(TALLOC_CTX *ctx, filename_in, NULL, NULL, - 0, + twrp, 0); if (smb_fname == NULL) { TALLOC_FREE(fname); @@ -1868,7 +1870,7 @@ char *get_original_lcomp(TALLOC_CTX *ctx, } status = canonicalize_snapshot_path(smb_fname, ucf_flags, - 0); + twrp); if (!NT_STATUS_IS_OK(status)) { TALLOC_FREE(fname); TALLOC_FREE(smb_fname); @@ -1928,10 +1930,12 @@ NTSTATUS filename_convert(TALLOC_CTX *ctx, if (ucf_flags & UCF_DFS_PATHNAME) { char *fname = NULL; + NTTIME dfs_twrp = 0; status = dfs_redirect(ctx, conn, name_in, ucf_flags, !conn->sconn->using_smb2, + &dfs_twrp, &fname); if (!NT_STATUS_IS_OK(status)) { DBG_DEBUG("dfs_redirect " @@ -1942,6 +1946,9 @@ NTSTATUS filename_convert(TALLOC_CTX *ctx, } name_in = fname; ucf_flags &= ~UCF_DFS_PATHNAME; + if (twrp == 0 && dfs_twrp != 0) { + twrp = dfs_twrp; + } } if (is_fake_file_path(name_in)) { diff --git a/source3/smbd/msdfs.c b/source3/smbd/msdfs.c index 86ca79a994b..07636592016 100644 --- a/source3/smbd/msdfs.c +++ b/source3/smbd/msdfs.c @@ -901,6 +901,7 @@ NTSTATUS dfs_redirect(TALLOC_CTX *ctx, const char *path_in, uint32_t ucf_flags, bool allow_broken_path, + NTTIME *_twrp, char **pp_path_out) { const struct loadparm_substitution *lp_sub = @@ -970,7 +971,7 @@ NTSTATUS dfs_redirect(TALLOC_CTX *ctx, path_in, pdp, ucf_flags, - NULL, /* twrp. */ + _twrp, /* twrp. */ NULL, /* int *consumedcntp */ NULL, /* struct referral **ppreflist */ NULL); /* size_t *preferral_count */ diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h index 5e7bc392b87..c1db119633e 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -567,6 +567,7 @@ NTSTATUS dfs_redirect(TALLOC_CTX *ctx, const char *name_in, uint32_t ucf_flags, bool allow_broken_path, + NTTIME *twrp, char **pp_name_out); struct connection_struct; struct smb_filename; -- 2.32.0 From fc1d7d773c13db03735f8821dda07dcee7da6c39 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 10:35:09 -0800 Subject: [PATCH 56/92] CVE-2021-44141: s3: smbd: Add filename_convert_smb1_search_path() - deals with SMB1 search pathnames. SMB1search and trans2 findfirst are unique in that they are the only passed in pathnames that can contain a terminal wildcard component. Deal with these two special cases with this new function that strips off the terminal wildcard and returns as the mask, and pass the non-wildcard parent directory component through the standard filename_convert(). Uses new helper function strip_gmt_from_raw_dfs(). When SMB1search and trans2 findfirst have been converted to use this function, we can strip all wildcard handling out of filename_convert() as we now know it will only ever be given valid pathnames. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/filename.c | 240 ++++++++++++++++++++++++++++++++++++++++ source3/smbd/proto.h | 6 + 2 files changed, 246 insertions(+) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index bcbe9f70015..ab2a418a3a0 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -2041,6 +2041,246 @@ NTSTATUS filename_convert(TALLOC_CTX *ctx, return status; } +/* + * Strip a @GMT component from an SMB1-DFS path. Could be anywhere + * in the path. + */ + +static char *strip_gmt_from_raw_dfs(TALLOC_CTX *ctx, + const char *name_in, + bool posix_pathnames, + NTTIME *_twrp) +{ + NTSTATUS status; + struct smb_filename *smb_fname = NULL; + char *name_out = NULL; + + smb_fname = synthetic_smb_fname(ctx, + name_in, + NULL, + NULL, + 0, + 0); + if (smb_fname == NULL) { + return NULL; + } + if (!posix_pathnames) { + /* + * Raw DFS names are still '\\' separated. + * canonicalize_snapshot_path() only works + * on '/' separated paths. Convert. + */ + string_replace(smb_fname->base_name, '\\', '/'); + } + status = canonicalize_snapshot_path(smb_fname, + UCF_GMT_PATHNAME, + 0); + if (!NT_STATUS_IS_OK(status)) { + TALLOC_FREE(smb_fname); + return NULL; + } + if (!posix_pathnames) { + /* Replace as raw DFS names. */ + string_replace(smb_fname->base_name, '/', '\\'); + } + name_out = talloc_strdup(ctx, smb_fname->base_name); + *_twrp = smb_fname->twrp; + TALLOC_FREE(smb_fname); + return name_out; +} + +/* + * Deal with the SMB1 semantics of sending a pathname with a + * wildcard as the terminal component for a SMB1search or + * trans2 findfirst. + */ + +NTSTATUS filename_convert_smb1_search_path(TALLOC_CTX *ctx, + connection_struct *conn, + const char *name_in, + uint32_t ucf_flags, + struct smb_filename **_smb_fname_out, + char **_mask_out) +{ + NTSTATUS status; + char *p = NULL; + char *mask = NULL; + struct smb_filename *smb_fname = NULL; + bool posix_pathnames = (ucf_flags & UCF_POSIX_PATHNAMES); + NTTIME twrp = 0; + TALLOC_CTX *frame = talloc_stackframe(); + + *_smb_fname_out = NULL; + *_mask_out = NULL; + + DBG_DEBUG("name_in: %s\n", name_in); + + if (ucf_flags & UCF_DFS_PATHNAME) { + /* + * We've been given a raw DFS pathname. + * In Windows mode this is separated by '\\' + * characters. + * + * We need to remove the last component + * which must be a wildcard before passing + * to dfs_redirect(). But the last component + * may also be a @GMT- token so we have to + * remove that first. + */ + char path_sep = posix_pathnames ? '/' : '\\'; + char *fname = NULL; + char *name_in_copy = NULL; + char *last_component = NULL; + + /* Work on a copy of name_in. */ + if (ucf_flags & UCF_GMT_PATHNAME) { + name_in_copy = strip_gmt_from_raw_dfs(frame, + name_in, + posix_pathnames, + &twrp); + ucf_flags &= ~UCF_GMT_PATHNAME; + } else { + name_in_copy = talloc_strdup(frame, name_in); + } + if (name_in_copy == NULL) { + TALLOC_FREE(frame); + return NT_STATUS_NO_MEMORY; + } + + /* + * Now we know that the last component is the + * wildcard. Copy it and truncate to remove it. + */ + p = strrchr_m(name_in_copy, path_sep); + if (p == NULL) { + last_component = talloc_strdup(frame, name_in_copy); + name_in_copy[0] = '\0'; + } else { + last_component = talloc_strdup(frame, p+1); + *p = '\0'; + } + if (last_component == NULL) { + TALLOC_FREE(frame); + return NT_STATUS_NO_MEMORY; + } + + DBG_DEBUG("name_in_copy: %s\n", name_in); + + /* + * Now we can call dfs_redirect() + * on the name without wildcard. + */ + status = dfs_redirect(frame, + conn, + name_in_copy, + ucf_flags, + !conn->sconn->using_smb2, + NULL, + &fname); + if (!NT_STATUS_IS_OK(status)) { + DBG_DEBUG("dfs_redirect " + "failed for name %s with %s\n", + name_in_copy, + nt_errstr(status)); + TALLOC_FREE(frame); + return status; + } + /* Add the last component back. */ + if (fname[0] == '\0') { + name_in = talloc_strdup(frame, last_component); + } else { + name_in = talloc_asprintf(frame, + "%s%c%s", + fname, + path_sep, + last_component); + } + if (name_in == NULL) { + TALLOC_FREE(frame); + return NT_STATUS_NO_MEMORY; + } + ucf_flags &= ~UCF_DFS_PATHNAME; + + DBG_DEBUG("After DFS redirect name_in: %s\n", name_in); + } + + smb_fname = synthetic_smb_fname(frame, + name_in, + NULL, + NULL, + twrp, + posix_pathnames ? + SMB_FILENAME_POSIX_PATH : 0); + if (smb_fname == NULL) { + TALLOC_FREE(frame); + return NT_STATUS_NO_MEMORY; + } + + /* Canonicalize any @GMT- paths. */ + status = canonicalize_snapshot_path(smb_fname, ucf_flags, twrp); + if (!NT_STATUS_IS_OK(status)) { + TALLOC_FREE(frame); + return status; + } + + /* Get the original lcomp. */ + mask = get_original_lcomp(frame, + conn, + name_in, + ucf_flags); + if (mask == NULL) { + TALLOC_FREE(frame); + return NT_STATUS_NO_MEMORY; + } + + if (mask[0] == '\0') { + /* Windows and OS/2 systems treat search on the root as * */ + TALLOC_FREE(mask); + mask = talloc_strdup(frame, "*"); + if (mask == NULL) { + TALLOC_FREE(frame); + return NT_STATUS_NO_MEMORY; + } + } + + DBG_DEBUG("mask = %s\n", mask); + + /* + * Remove the terminal component so + * filename_convert never sees the mask. + */ + p = strrchr_m(smb_fname->base_name,'/'); + if (p == NULL) { + /* filename_convert handles a '\0' base_name. */ + smb_fname->base_name[0] = '\0'; + } else { + *p = '\0'; + } + + DBG_DEBUG("For filename_convert: smb_fname = %s\n", + smb_fname_str_dbg(smb_fname)); + + /* Convert the parent directory path. */ + status = filename_convert(frame, + conn, + smb_fname->base_name, + ucf_flags, + smb_fname->twrp, + &smb_fname); + + if (NT_STATUS_IS_OK(status)) { + *_smb_fname_out = talloc_move(ctx, &smb_fname); + *_mask_out = talloc_move(ctx, &mask); + } else { + DBG_DEBUG("filename_convert error for %s: %s\n", + smb_fname_str_dbg(smb_fname), + nt_errstr(status)); + } + + TALLOC_FREE(frame); + return status; +} + /* * Build the full path from a dirfsp and dirfsp relative name */ diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h index c1db119633e..46d3f2e5156 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -382,6 +382,12 @@ NTSTATUS filename_convert(TALLOC_CTX *mem_ctx, uint32_t ucf_flags, NTTIME twrp, struct smb_filename **pp_smb_fname); +NTSTATUS filename_convert_smb1_search_path(TALLOC_CTX *ctx, + connection_struct *conn, + const char *name_in, + uint32_t ucf_flags, + struct smb_filename **_smb_fname_out, + char **_mask_out); /* The following definitions come from smbd/files.c */ -- 2.32.0 From 51553b94ba8d5a5f4bcc9a6afe084c8c45412a52 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 11:22:03 -0800 Subject: [PATCH 57/92] CVE-2021-44141: s3: smbd: Convert reply_search() to use filename_convert_smb1_search_path(). Cleans up this code path nicely ! BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/reply.c | 71 ++++++++++---------------------------------- 1 file changed, 16 insertions(+), 55 deletions(-) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index d2048856cff..cc95bfa3af0 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -1747,15 +1747,17 @@ void reply_search(struct smb_request *req) /* dirtype &= ~FILE_ATTRIBUTE_DIRECTORY; */ if (status_len == 0) { - int ret; + const char *dirpath; struct smb_filename *smb_dname = NULL; - uint32_t ucf_flags = UCF_ALWAYS_ALLOW_WCARD_LCOMP | - ucf_flags_from_smb_request(req); - nt_status = filename_convert(ctx, conn, - path, - ucf_flags, - 0, - &smb_fname); + uint32_t ucf_flags = ucf_flags_from_smb_request(req); + + nt_status = filename_convert_smb1_search_path(ctx, + conn, + path, + ucf_flags, + &smb_dname, + &mask); + if (!NT_STATUS_IS_OK(nt_status)) { if (NT_STATUS_EQUAL(nt_status,NT_STATUS_PATH_NOT_COVERED)) { reply_botherror(req, NT_STATUS_PATH_NOT_COVERED, @@ -1766,56 +1768,9 @@ void reply_search(struct smb_request *req) goto out; } - directory = smb_fname->base_name; - - p = strrchr_m(directory,'/'); - if ((p != NULL) && (*directory != '/')) { - mask = talloc_strdup(ctx, p + 1); - directory = talloc_strndup(ctx, directory, - PTR_DIFF(p, directory)); - } else { - mask = talloc_strdup(ctx, directory); - directory = talloc_strdup(ctx,"."); - } - - if (!directory) { - reply_nterror(req, NT_STATUS_NO_MEMORY); - goto out; - } - memset((char *)status,'\0',21); SCVAL(status,0,(dirtype & 0x1F)); - smb_dname = synthetic_smb_fname(talloc_tos(), - directory, - NULL, - NULL, - smb_fname->twrp, - smb_fname->flags); - if (smb_dname == NULL) { - reply_nterror(req, NT_STATUS_NO_MEMORY); - goto out; - } - - /* - * As we've cut off the last component from - * smb_fname we need to re-stat smb_dname - * so FILE_OPEN disposition knows the directory - * exists. - */ - ret = vfs_stat(conn, smb_dname); - if (ret == -1) { - nt_status = map_nt_error_from_unix(errno); - reply_nterror(req, nt_status); - goto out; - } - - nt_status = openat_pathref_fsp(conn->cwd_fsp, smb_dname); - if (!NT_STATUS_IS_OK(nt_status)) { - reply_nterror(req, nt_status); - goto out; - } - /* * Open an fsp on this directory for the dptr. */ @@ -1872,6 +1827,12 @@ void reply_search(struct smb_request *req) } dptr_num = dptr_dnum(fsp->dptr); + dirpath = dptr_path(sconn, dptr_num); + directory = talloc_strdup(ctx, dirpath); + if (!directory) { + reply_nterror(req, NT_STATUS_NO_MEMORY); + goto out; + } } else { int status_dirtype; -- 2.32.0 From 2976bb613a5b15cf26519d5fbcf73dd4e87de393 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 11:28:40 -0800 Subject: [PATCH 58/92] CVE-2021-44141: s3: smbd: Fix call_trans2findfirst() to use filename_convert_smb1_search_path(). filename_convert() no longer has to handle wildcards. UCF_ALWAYS_ALLOW_WCARD_LCOMP is now unused. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/trans2.c | 99 ++++--------------------------------------- 1 file changed, 8 insertions(+), 91 deletions(-) diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c index 06077a87dad..0de070c5d08 100644 --- a/source3/smbd/trans2.c +++ b/source3/smbd/trans2.c @@ -2663,14 +2663,12 @@ static void call_trans2findfirst(connection_struct *conn, NTSTATUS ntstatus = NT_STATUS_OK; bool ask_sharemode = lp_smbd_search_ask_sharemode(SNUM(conn)); struct smbd_server_connection *sconn = req->sconn; - uint32_t ucf_flags = UCF_ALWAYS_ALLOW_WCARD_LCOMP | - ucf_flags_from_smb_request(req); + uint32_t ucf_flags = ucf_flags_from_smb_request(req); bool backup_priv = false; bool as_root = false; files_struct *fsp = NULL; const struct loadparm_substitution *lp_sub = loadparm_s3_global_substitution(); - int ret; if (total_params < 13) { reply_nterror(req, NT_STATUS_INVALID_PARAMETER); @@ -2756,11 +2754,12 @@ close_if_end = %d requires_resume_key = %d backup_priv = %d level = 0x%x, max_da become_root(); as_root = true; } - ntstatus = filename_convert(talloc_tos(), conn, - directory, - ucf_flags, - 0, - &smb_dname); + ntstatus = filename_convert_smb1_search_path(talloc_tos(), + conn, + directory, + ucf_flags, + &smb_dname, + &mask); if (!NT_STATUS_IS_OK(ntstatus)) { if (NT_STATUS_EQUAL(ntstatus,NT_STATUS_PATH_NOT_COVERED)) { @@ -2772,72 +2771,9 @@ close_if_end = %d requires_resume_key = %d backup_priv = %d level = 0x%x, max_da goto out; } - /* - * The above call to filename_convert() is on the path from the client - * including the search mask. Until the code that chops of the search - * mask from the path below is moved before the call to - * filename_convert(), we close a possible pathref fsp to ensure - * SMB_VFS_CREATE_FILE() below will internally open a pathref fsp on the - * correct path. - */ - if (smb_dname->fsp != NULL) { - ntstatus = fd_close(smb_dname->fsp); - if (!NT_STATUS_IS_OK(ntstatus)) { - reply_nterror(req, ntstatus); - goto out; - } - /* - * The pathref fsp link destructor will set smb_dname->fsp to - * NULL. Turning this into an assert to give a hint at readers - * of the code trying to understand the mechanics. - */ - file_free(req, smb_dname->fsp); - SMB_ASSERT(smb_dname->fsp == NULL); - } - - mask = get_original_lcomp(talloc_tos(), - conn, - directory, - ucf_flags); - if (mask == NULL) { - reply_nterror(req, NT_STATUS_NO_MEMORY); - goto out; - } - + TALLOC_FREE(directory); directory = smb_dname->base_name; - p = strrchr_m(directory,'/'); - if(p == NULL) { - /* Windows and OS/2 systems treat search on the root '\' as if it were '\*' */ - if((directory[0] == '.') && (directory[1] == '\0')) { - mask = talloc_strdup(talloc_tos(),"*"); - if (!mask) { - reply_nterror(req, NT_STATUS_NO_MEMORY); - goto out; - } - } - } else { - *p = 0; - } - - if (p == NULL || p == directory) { - struct smb_filename *old_name = smb_dname; - - /* Ensure we don't have a directory name of "". */ - smb_dname = synthetic_smb_fname(talloc_tos(), - ".", - NULL, - &old_name->st, - old_name->twrp, - old_name->flags); - TALLOC_FREE(old_name); - if (smb_dname == NULL) { - reply_nterror(req, NT_STATUS_NO_MEMORY); - goto out; - } - directory = smb_dname->base_name; - } - DEBUG(5,("dir=%s, mask = %s\n",directory, mask)); if (info_level == SMB_FIND_EA_LIST) { @@ -2895,25 +2831,6 @@ total_data=%u (should be %u)\n", (unsigned int)total_data, (unsigned int)IVAL(pd } params = *pparams; - /* - * As we've cut off the last component from - * smb_fname we need to re-stat smb_dname - * so FILE_OPEN disposition knows the directory - * exists. - */ - ret = vfs_stat(conn, smb_dname); - if (ret == -1) { - ntstatus = map_nt_error_from_unix(errno); - reply_nterror(req, ntstatus); - goto out; - } - - ntstatus = openat_pathref_fsp(conn->cwd_fsp, smb_dname); - if (!NT_STATUS_IS_OK(ntstatus)) { - reply_nterror(req, ntstatus); - goto out; - } - /* * Open an fsp on this directory for the dptr. */ -- 2.32.0 From a32da4561d5ab1d6b80cfa70602569c8f9be8ac2 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 11:31:40 -0800 Subject: [PATCH 59/92] CVE-2021-44141: s3: smbd: dfs_path_lookup() no longer deals with wildcards. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/msdfs.c | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/source3/smbd/msdfs.c b/source3/smbd/msdfs.c index 07636592016..6a3abd2f6eb 100644 --- a/source3/smbd/msdfs.c +++ b/source3/smbd/msdfs.c @@ -680,10 +680,6 @@ bool is_msdfs_link(struct files_struct *dirfsp, Used by other functions to decide if a dfs path is remote, and to get the list of referred locations for that remote path. - search_flag: For findfirsts, dfs links themselves are not - redirected, but paths beyond the links are. For normal smb calls, - even dfs links need to be redirected. - consumedcntp: how much of the dfs path is being redirected. the client should try the remaining path on the redirected server. @@ -755,15 +751,6 @@ static NTSTATUS dfs_path_lookup(TALLOC_CTX *ctx, TALLOC_FREE(parent_fname); if (NT_STATUS_IS_OK(status)) { - /* XX_ALLOW_WCARD_XXX is called from search functions.*/ - if (ucf_flags & UCF_ALWAYS_ALLOW_WCARD_LCOMP) { - DBG_INFO("(FindFirst) No redirection " - "for dfs link %s.\n", - dfspath); - status = NT_STATUS_OK; - goto out; - } - DBG_INFO("%s resolves to a valid dfs link\n", dfspath); -- 2.32.0 From a48a7c453aae8394dca20c55ddd91b874fd7aa5a Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 11:42:23 -0800 Subject: [PATCH 60/92] CVE-2021-44141: s3: smbd: Remove 'bool search_wcard_flag' from parse_dfs_path(). Never set. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/msdfs.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/source3/smbd/msdfs.c b/source3/smbd/msdfs.c index 6a3abd2f6eb..1feeec6fff2 100644 --- a/source3/smbd/msdfs.c +++ b/source3/smbd/msdfs.c @@ -894,14 +894,13 @@ NTSTATUS dfs_redirect(TALLOC_CTX *ctx, const struct loadparm_substitution *lp_sub = loadparm_s3_global_substitution(); NTSTATUS status; - bool search_wcard_flag = (ucf_flags & UCF_ALWAYS_ALLOW_WCARD_LCOMP); struct dfs_path *pdp = talloc(ctx, struct dfs_path); if (!pdp) { return NT_STATUS_NO_MEMORY; } - status = parse_dfs_path(conn, path_in, search_wcard_flag, + status = parse_dfs_path(conn, path_in, false, allow_broken_path, pdp); if (!NT_STATUS_IS_OK(status)) { TALLOC_FREE(pdp); -- 2.32.0 From eaedf1e2a57f762bd203dcd6fa12f39577638fac Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 11:48:23 -0800 Subject: [PATCH 61/92] CVE-2021-44141: s3: smbd: parse_dfs_path() can ignore wildcards. If one is passed to filename_convert(), it will error out there with NT_STATUS_OBJECT_NAME_INVALID. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/msdfs.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/source3/smbd/msdfs.c b/source3/smbd/msdfs.c index 1feeec6fff2..c003b442baa 100644 --- a/source3/smbd/msdfs.c +++ b/source3/smbd/msdfs.c @@ -215,12 +215,6 @@ static NTSTATUS parse_dfs_path(connection_struct *conn, if (pdp->posix_path) { status = check_path_syntax_posix(pdp->reqpath); } else { - if (!allow_wcards) { - bool has_wcard = ms_has_wild(pdp->reqpath); - if (has_wcard) { - return NT_STATUS_INVALID_PARAMETER; - } - } status = check_path_syntax(pdp->reqpath); } -- 2.32.0 From f71ae0dc0a6cdff67894733e3b63348b4f5898b3 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 11:30:42 -0800 Subject: [PATCH 62/92] CVE-2021-44141: s3: smbd: filename_convert() no longer deals with wildcards. These are already errored out with NT_STATUS_OBJECT_NAME_INVALID in the unix_convert() code. Remove the check. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/filename.c | 9 --------- 1 file changed, 9 deletions(-) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index ab2a418a3a0..dca5de31be6 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -1923,7 +1923,6 @@ NTSTATUS filename_convert(TALLOC_CTX *ctx, struct smb_filename **_smb_fname) { struct smb_filename *smb_fname = NULL; - bool has_wild; NTSTATUS status; *_smb_fname = NULL; @@ -1998,14 +1997,6 @@ NTSTATUS filename_convert(TALLOC_CTX *ctx, return status; } - has_wild = ms_has_wild(name_in); - if (has_wild) { - DBG_DEBUG("[%s] contains wildcard, skipping pathref fsp\n", - name_in); - *_smb_fname = smb_fname; - return NT_STATUS_OK; - } - if (!VALID_STAT(smb_fname->st)) { DBG_DEBUG("[%s] does not exist, skipping pathref fsp\n", smb_fname_str_dbg(smb_fname)); -- 2.32.0 From 9d99b8c33c2a712c73f0d5ad760ca973352c2639 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 11:33:42 -0800 Subject: [PATCH 63/92] CVE-2021-44141: s3: smbd: Inside 'struct uc_state', remove allow_wcard_last_component. This is never allowed. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/filename.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index dca5de31be6..dc2a5a33824 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -524,7 +524,6 @@ struct uc_state { bool component_was_mangled; bool name_has_wildcard; bool posix_pathnames; - bool allow_wcard_last_component; bool done; }; @@ -870,7 +869,7 @@ static NTSTATUS unix_convert_step(struct uc_state *state) return NT_STATUS_OBJECT_NAME_INVALID; } return determine_path_error(state->end+1, - state->allow_wcard_last_component, + false, state->posix_pathnames); } @@ -962,7 +961,6 @@ NTSTATUS unix_convert(TALLOC_CTX *mem_ctx, .orig_path = orig_path, .ucf_flags = ucf_flags, .posix_pathnames = (ucf_flags & UCF_POSIX_PATHNAMES), - .allow_wcard_last_component = (ucf_flags & UCF_ALWAYS_ALLOW_WCARD_LCOMP), }; *smb_fname_out = NULL; @@ -1038,7 +1036,7 @@ NTSTATUS unix_convert(TALLOC_CTX *mem_ctx, status = NT_STATUS_OBJECT_NAME_INVALID; } else { status =determine_path_error(&state->orig_path[2], - state->allow_wcard_last_component, + false, state->posix_pathnames); } goto err; @@ -1163,7 +1161,7 @@ NTSTATUS unix_convert(TALLOC_CTX *mem_ctx, if (!state->posix_pathnames) { /* POSIX pathnames have no wildcards. */ state->name_has_wildcard = ms_has_wild(state->smb_fname->base_name); - if (state->name_has_wildcard && !state->allow_wcard_last_component) { + if (state->name_has_wildcard) { /* Wildcard not valid anywhere. */ status = NT_STATUS_OBJECT_NAME_INVALID; goto fail; -- 2.32.0 From 1f4ed5108922a0dbf171d605f968f2f2cf2e8874 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 12:37:15 -0800 Subject: [PATCH 64/92] CVE-2021-44141: s3: smbd: We no longer need determine_path_error(). Now we don't have to consider wildcards just return NT_STATUS_OBJECT_PATH_NOT_FOUND for the cases we used to call it. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/filename.c | 47 ++--------------------------------------- 1 file changed, 2 insertions(+), 45 deletions(-) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index dc2a5a33824..09bf859bd2d 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -90,45 +90,6 @@ static bool mangled_equal(const char *name1, return strequal(name1, mname); } -/**************************************************************************** - Cope with the differing wildcard and non-wildcard error cases. -****************************************************************************/ - -static NTSTATUS determine_path_error(const char *name, - bool allow_wcard_last_component, - bool posix_pathnames) -{ - const char *p; - bool name_has_wild = false; - - if (!allow_wcard_last_component) { - /* Error code within a pathname. */ - return NT_STATUS_OBJECT_PATH_NOT_FOUND; - } - - /* We're terminating here so we - * can be a little slower and get - * the error code right. Windows - * treats the last part of the pathname - * separately I think, so if the last - * component is a wildcard then we treat - * this ./ as "end of component" */ - - p = strchr(name, '/'); - - if (!posix_pathnames) { - name_has_wild = ms_has_wild(name); - } - - if (!p && (name_has_wild || ISDOT(name))) { - /* Error code at the end of a pathname. */ - return NT_STATUS_OBJECT_NAME_INVALID; - } else { - /* Error code within a pathname. */ - return NT_STATUS_OBJECT_PATH_NOT_FOUND; - } -} - static NTSTATUS check_for_dot_component(const struct smb_filename *smb_fname) { /* Ensure we catch all names with in "/." @@ -868,9 +829,7 @@ static NTSTATUS unix_convert_step(struct uc_state *state) /* Error code at the end of a pathname. */ return NT_STATUS_OBJECT_NAME_INVALID; } - return determine_path_error(state->end+1, - false, - state->posix_pathnames); + return NT_STATUS_OBJECT_PATH_NOT_FOUND; } /* The name cannot have a wildcard if it's not @@ -1035,9 +994,7 @@ NTSTATUS unix_convert(TALLOC_CTX *mem_ctx, if (state->orig_path[1] == '\0' || state->orig_path[2] == '\0') { status = NT_STATUS_OBJECT_NAME_INVALID; } else { - status =determine_path_error(&state->orig_path[2], - false, - state->posix_pathnames); + status = NT_STATUS_OBJECT_PATH_NOT_FOUND; } goto err; } -- 2.32.0 From 8c394dda74d45eb6584a8c83ff50d1a2305b9a6f Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 12:40:43 -0800 Subject: [PATCH 65/92] CVE-2021-44141: s3: smbd: UCF_ALWAYS_ALLOW_WCARD_LCOMP 0x00000002 is no longer used. Hurrah ! BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/filename.c | 3 --- source3/smbd/smbd.h | 2 +- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index 09bf859bd2d..73a2f116915 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -459,9 +459,6 @@ Note NT_STATUS_OK doesn't mean the name exists or is valid, just that we didn't get any fatal errors that should immediately terminate the calling SMB processing whilst resolving. -If UCF_ALWAYS_ALLOW_WCARD_LCOMP is passed in, then a MS wildcard -should be allowed in the last component of the path only. - If the orig_path was a stream, smb_filename->base_name will point to the base filename, and smb_filename->stream_name will point to the stream name. If orig_path was not a stream, then smb_filename->stream_name will be NULL. diff --git a/source3/smbd/smbd.h b/source3/smbd/smbd.h index 84fcf033a71..88bff2830d9 100644 --- a/source3/smbd/smbd.h +++ b/source3/smbd/smbd.h @@ -61,7 +61,7 @@ struct trans_state { * unix_convert_flags */ /* UCF_SAVE_LCOMP 0x00000001 is no longer used. */ -#define UCF_ALWAYS_ALLOW_WCARD_LCOMP 0x00000002 +/* UCF_ALWAYS_ALLOW_WCARD_LCOMP 0x00000002 is no longer used. */ /* UCF_COND_ALLOW_WCARD_LCOMP 0x00000004 is no longer used. */ #define UCF_POSIX_PATHNAMES 0x00000008 /* #define UCF_UNIX_NAME_LOOKUP 0x00000010 is no longer used. */ -- 2.32.0 From fc0fdbf1a6b27ca04a48610603389a58dbaf4cb5 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 12:53:36 -0800 Subject: [PATCH 66/92] CVE-2021-44141: s3: smbd: Inside unix_convert(), never set state->name_is_wildcard. We error out immediately if it's set anyway. Preparing to remove 'state->name_is_wildcard' structure element. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/filename.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index 73a2f116915..2095c23aaf4 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -1114,8 +1114,8 @@ NTSTATUS unix_convert(TALLOC_CTX *mem_ctx, if (!state->posix_pathnames) { /* POSIX pathnames have no wildcards. */ - state->name_has_wildcard = ms_has_wild(state->smb_fname->base_name); - if (state->name_has_wildcard) { + bool name_has_wildcard = ms_has_wild(state->smb_fname->base_name); + if (name_has_wildcard) { /* Wildcard not valid anywhere. */ status = NT_STATUS_OBJECT_NAME_INVALID; goto fail; -- 2.32.0 From 190b95336bac3175caf5d2374591dc4c67ae7ee6 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 12:55:41 -0800 Subject: [PATCH 67/92] CVE-2021-44141: s3: smbd: In unix_convert(), remove all references to state->name_has_wildcard. It is never set. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/filename.c | 25 ++----------------------- 1 file changed, 2 insertions(+), 23 deletions(-) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index 2095c23aaf4..01c0d9d7dbd 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -1125,7 +1125,7 @@ NTSTATUS unix_convert(TALLOC_CTX *mem_ctx, DBG_DEBUG("Begin: name [%s] dirpath [%s] name [%s]\n", state->smb_fname->base_name, state->dirpath, state->name); - if (!state->name_has_wildcard) { + { int parent_stat_errno = 0; /* @@ -1239,27 +1239,6 @@ NTSTATUS unix_convert(TALLOC_CTX *mem_ctx, goto done; } } - } else { - /* - * We have a wildcard in the pathname. - * - * Optimization for common case where the wildcard - * is in the last component and the client already - * sent the correct case. - * NOTE : check_parent_exists() doesn't preserve errno. - */ - int saved_errno = errno; - status = check_parent_exists(state->mem_ctx, - state->conn, - state->posix_pathnames, - state->smb_fname, - &state->dirpath, - &state->name, - NULL); - errno = saved_errno; - if (!NT_STATUS_IS_OK(status)) { - goto fail; - } } /* @@ -1299,7 +1278,7 @@ NTSTATUS unix_convert(TALLOC_CTX *mem_ctx, * components as this can change the size. */ - if(!state->component_was_mangled && !state->name_has_wildcard) { + if(!state->component_was_mangled) { stat_cache_add(state->orig_path, state->smb_fname->base_name, state->smb_fname->twrp, -- 2.32.0 From 9ff540238963cdedd4c73e3f8e6c02eba4b941eb Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 12:59:50 -0800 Subject: [PATCH 68/92] CVE-2021-44141: s3: smbd: In unix_convert() remove the now unneeded block indentation. We removed the 'if (state->name_has_wildcard) {' clause, so the block no longer needs indenting. Best seen with git show -b. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/filename.c | 193 ++++++++++++++++++++-------------------- 1 file changed, 95 insertions(+), 98 deletions(-) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index 01c0d9d7dbd..c8b3f1f5955 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -910,6 +910,7 @@ NTSTATUS unix_convert(TALLOC_CTX *mem_ctx, struct uc_state *state = &uc_state; NTSTATUS status; int ret = -1; + int parent_stat_errno = 0; *state = (struct uc_state) { .mem_ctx = mem_ctx, @@ -1125,119 +1126,115 @@ NTSTATUS unix_convert(TALLOC_CTX *mem_ctx, DBG_DEBUG("Begin: name [%s] dirpath [%s] name [%s]\n", state->smb_fname->base_name, state->dirpath, state->name); - { - int parent_stat_errno = 0; + /* + * stat the name - if it exists then we can add the stream back (if + * there was one) and be done! + */ - /* - * stat the name - if it exists then we can add the stream back (if - * there was one) and be done! - */ + ret = vfs_stat(state->conn, state->smb_fname); + if (ret == 0) { + status = check_for_dot_component(state->smb_fname); + if (!NT_STATUS_IS_OK(status)) { + goto fail; + } + /* Add the path (not including the stream) to the cache. */ + stat_cache_add(state->orig_path, + state->smb_fname->base_name, + state->smb_fname->twrp, + state->conn->case_sensitive); + DBG_DEBUG("Conversion of base_name finished " + "[%s] -> [%s]\n", + state->orig_path, state->smb_fname->base_name); + goto done; + } - ret = vfs_stat(state->conn, state->smb_fname); - if (ret == 0) { - status = check_for_dot_component(state->smb_fname); - if (!NT_STATUS_IS_OK(status)) { - goto fail; - } - /* Add the path (not including the stream) to the cache. */ - stat_cache_add(state->orig_path, - state->smb_fname->base_name, - state->smb_fname->twrp, - state->conn->case_sensitive); - DBG_DEBUG("Conversion of base_name finished " - "[%s] -> [%s]\n", - state->orig_path, state->smb_fname->base_name); - goto done; + /* Stat failed - ensure we don't use it. */ + SET_STAT_INVALID(state->smb_fname->st); + + /* + * Note: we must continue processing a path if we get EACCES + * from stat. With NFS4 permissions the file might be lacking + * READ_ATTR, but if the parent has LIST permissions we can + * resolve the path in the path traversal loop down below. + */ + + if (errno == ENOENT) { + /* Optimization when creating a new file - only + the last component doesn't exist. + NOTE : check_parent_exists() doesn't preserve errno. + */ + int saved_errno = errno; + status = check_parent_exists(state->mem_ctx, + state->conn, + state->posix_pathnames, + state->smb_fname, + &state->dirpath, + &state->name, + &parent_stat_errno); + errno = saved_errno; + if (!NT_STATUS_IS_OK(status)) { + goto fail; } + } - /* Stat failed - ensure we don't use it. */ - SET_STAT_INVALID(state->smb_fname->st); + /* + * A special case - if we don't have any wildcards or mangling chars and are case + * sensitive or the underlying filesystem is case insensitive then searching + * won't help. + */ - /* - * Note: we must continue processing a path if we get EACCES - * from stat. With NFS4 permissions the file might be lacking - * READ_ATTR, but if the parent has LIST permissions we can - * resolve the path in the path traversal loop down below. - */ + if ((state->conn->case_sensitive || !(state->conn->fs_capabilities & + FILE_CASE_SENSITIVE_SEARCH)) && + !mangle_is_mangled(state->smb_fname->base_name, state->conn->params)) { - if (errno == ENOENT) { - /* Optimization when creating a new file - only - the last component doesn't exist. - NOTE : check_parent_exists() doesn't preserve errno. - */ - int saved_errno = errno; - status = check_parent_exists(state->mem_ctx, - state->conn, - state->posix_pathnames, - state->smb_fname, - &state->dirpath, - &state->name, - &parent_stat_errno); - errno = saved_errno; - if (!NT_STATUS_IS_OK(status)) { - goto fail; - } + status = check_for_dot_component(state->smb_fname); + if (!NT_STATUS_IS_OK(status)) { + goto fail; } /* - * A special case - if we don't have any wildcards or mangling chars and are case - * sensitive or the underlying filesystem is case insensitive then searching - * won't help. + * The stat failed. Could be ok as it could be + * a new file. */ - if ((state->conn->case_sensitive || !(state->conn->fs_capabilities & - FILE_CASE_SENSITIVE_SEARCH)) && - !mangle_is_mangled(state->smb_fname->base_name, state->conn->params)) { - - status = check_for_dot_component(state->smb_fname); - if (!NT_STATUS_IS_OK(status)) { - goto fail; - } - + if (errno == ENOTDIR || errno == ELOOP) { + status = NT_STATUS_OBJECT_PATH_NOT_FOUND; + goto fail; + } else if (errno == ENOENT) { /* - * The stat failed. Could be ok as it could be - * a new file. - */ - - if (errno == ENOTDIR || errno == ELOOP) { + * Was it a missing last component ? + * or a missing intermediate component ? + * + * Optimization. + * + * For this code path we can guarantee that + * we have gone through check_parent_exists() + * and it returned NT_STATUS_OK. + * + * Either there was no parent component (".") + * parent_stat_errno == 0 and we have a missing + * last component here. + * + * OR check_parent_exists() called STAT/LSTAT + * and if it failed parent_stat_errno has been + * set telling us if the parent existed or not. + * + * Either way we can avoid another STAT/LSTAT + * system call on the parent here. + */ + if (parent_stat_errno == ENOTDIR || + parent_stat_errno == ENOENT || + parent_stat_errno == ELOOP) { status = NT_STATUS_OBJECT_PATH_NOT_FOUND; goto fail; - } else if (errno == ENOENT) { - /* - * Was it a missing last component ? - * or a missing intermediate component ? - * - * Optimization. - * - * For this code path we can guarantee that - * we have gone through check_parent_exists() - * and it returned NT_STATUS_OK. - * - * Either there was no parent component (".") - * parent_stat_errno == 0 and we have a missing - * last component here. - * - * OR check_parent_exists() called STAT/LSTAT - * and if it failed parent_stat_errno has been - * set telling us if the parent existed or not. - * - * Either way we can avoid another STAT/LSTAT - * system call on the parent here. - */ - if (parent_stat_errno == ENOTDIR || - parent_stat_errno == ENOENT || - parent_stat_errno == ELOOP) { - status = NT_STATUS_OBJECT_PATH_NOT_FOUND; - goto fail; - } - - /* - * Missing last component is ok - new file. - * Also deal with permission denied elsewhere. - * Just drop out to done. - */ - goto done; } + + /* + * Missing last component is ok - new file. + * Also deal with permission denied elsewhere. + * Just drop out to done. + */ + goto done; } } -- 2.32.0 From 7d164fa32109b04b9d35db24f5ea0d26f214ef94 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 13:03:47 -0800 Subject: [PATCH 69/92] CVE-2021-44141: s3: smbd: In unix_convert_step() remove all use of 'state->name_was_wildcard' We know it is never true. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/filename.c | 23 ++--------------------- 1 file changed, 2 insertions(+), 21 deletions(-) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index c8b3f1f5955..c582d24ec6b 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -829,25 +829,6 @@ static NTSTATUS unix_convert_step(struct uc_state *state) return NT_STATUS_OBJECT_PATH_NOT_FOUND; } - /* The name cannot have a wildcard if it's not - the last component. */ - - if (!state->posix_pathnames) { - state->name_has_wildcard = ms_has_wild(state->name); - } - - /* Wildcards never valid within a pathname. */ - if (state->name_has_wildcard && state->end != NULL) { - return NT_STATUS_OBJECT_NAME_INVALID; - } - - /* Skip the stat call if it's a wildcard end. */ - if (state->name_has_wildcard) { - DBG_DEBUG("Wildcard [%s]\n", state->name); - state->done = true; - return NT_STATUS_OK; - } - status = unix_convert_step_stat(state); if (!NT_STATUS_IS_OK(status)) { return status; @@ -880,9 +861,9 @@ static NTSTATUS unix_convert_step(struct uc_state *state) /* * Cache the dirpath thus far. Don't cache a name with mangled - * or wildcard components as this can change the size. + * components as this can change the size. */ - if(!state->component_was_mangled && !state->name_has_wildcard) { + if(!state->component_was_mangled) { stat_cache_add(state->orig_path, state->dirpath, state->smb_fname->twrp, -- 2.32.0 From 7c28f85e5170f305ca791043dcaea51756b6a508 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 13:05:55 -0800 Subject: [PATCH 70/92] CVE-2021-44141: s3: smbd: In unix_convert_step_stat() remove use of state->name_was_wildcard. It can never be true. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/filename.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index c582d24ec6b..8f7da13f034 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -662,8 +662,8 @@ static NTSTATUS unix_convert_step_stat(struct uc_state *state) if (state->posix_pathnames) { /* * For posix_pathnames, we're done. - * Don't blunder into the name_has_wildcard OR - * get_real_filename() codepaths as they may + * Don't blunder into the + * get_real_filename() codepath as they may * be doing case insensitive lookups. So when * creating a new POSIX directory Foo they might * match on name foo. @@ -712,10 +712,6 @@ static NTSTATUS unix_convert_step_stat(struct uc_state *state) * Try to find this part of the path in the directory. */ - if (state->name_has_wildcard) { - return unix_convert_step_search_fail(state); - } - dname = (struct smb_filename) { .base_name = state->dirpath, .twrp = state->smb_fname->twrp, -- 2.32.0 From 482a2000131669dda8674767a1d3a8477e8c2c66 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 3 Dec 2021 13:06:27 -0800 Subject: [PATCH 71/92] CVE-2021-44141: s3: smbd: Remove 'struct uc_state' name_has_wildcard element. It is never set or looked at. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/smbd/filename.c | 1 - 1 file changed, 1 deletion(-) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index 8f7da13f034..0d82085870c 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -480,7 +480,6 @@ struct uc_state { char *dirpath; char *stream; bool component_was_mangled; - bool name_has_wildcard; bool posix_pathnames; bool done; }; -- 2.32.0 From 4896bef12cd18176fd9e35a374a5553a0906e652 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 19 Nov 2021 12:54:47 -0800 Subject: [PATCH 72/92] CVE-2021-44141: s4: torture: Fix raw.search:test_one_file() to use torture_result() instead of printf. I think this test pre-dates torture_result. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source4/torture/raw/search.c | 60 ++++++++++++++++++++++++------------ 1 file changed, 41 insertions(+), 19 deletions(-) diff --git a/source4/torture/raw/search.c b/source4/torture/raw/search.c index 14af01bffad..07285893f40 100644 --- a/source4/torture/raw/search.c +++ b/source4/torture/raw/search.c @@ -316,7 +316,11 @@ static bool test_one_file(struct torture_context *tctx, fnum = create_complex_file(cli, tctx, fname); if (fnum == -1) { - printf("ERROR: open of %s failed (%s)\n", fname, smbcli_errstr(cli->tree)); + torture_result(tctx, + TORTURE_FAIL, + __location__"ERROR: open of %s failed (%s)\n", + fname, + smbcli_errstr(cli->tree)); ret = false; goto done; } @@ -343,9 +347,11 @@ static bool test_one_file(struct torture_context *tctx, } if (!NT_STATUS_IS_OK(levels[i].status)) { - printf("search level %s(%d) failed - %s\n", - levels[i].name, (int)levels[i].level, - nt_errstr(levels[i].status)); + torture_result(tctx, + TORTURE_FAIL, + __location__"search level %s(%d) failed - %s\n", + levels[i].name, (int)levels[i].level, + nt_errstr(levels[i].status)); ret = false; continue; } @@ -363,7 +369,9 @@ static bool test_one_file(struct torture_context *tctx, expected_status = STATUS_NO_MORE_FILES; } if (!NT_STATUS_EQUAL(status, expected_status)) { - printf("search level %s(%d) should fail with %s - %s\n", + torture_result(tctx, + TORTURE_FAIL, + __location__"search level %s(%d) should fail with %s - %s\n", levels[i].name, (int)levels[i].level, nt_errstr(expected_status), nt_errstr(status)); @@ -400,8 +408,10 @@ static bool test_one_file(struct torture_context *tctx, s = find(name); \ if (s) { \ if ((s->sname1.field1) != (v.sname2.out.field2)) { \ - printf("(%s) %s/%s [0x%x] != %s/%s [0x%x]\n", \ - __location__, \ + torture_result(tctx,\ + TORTURE_FAIL,\ + "(%s) %s/%s [0x%x] != %s/%s [0x%x]\n", \ + __location__, \ #sname1, #field1, (int)s->sname1.field1, \ #sname2, #field2, (int)v.sname2.out.field2); \ ret = false; \ @@ -412,8 +422,10 @@ static bool test_one_file(struct torture_context *tctx, s = find(name); \ if (s) { \ if (s->sname1.field1 != (~1 & nt_time_to_unix(v.sname2.out.field2))) { \ - printf("(%s) %s/%s [%s] != %s/%s [%s]\n", \ - __location__, \ + torture_result(tctx,\ + TORTURE_FAIL,\ + "(%s) %s/%s [%s] != %s/%s [%s]\n", \ + __location__, \ #sname1, #field1, timestring(tctx, s->sname1.field1), \ #sname2, #field2, nt_time_string(tctx, v.sname2.out.field2)); \ ret = false; \ @@ -424,8 +436,10 @@ static bool test_one_file(struct torture_context *tctx, s = find(name); \ if (s) { \ if (s->sname1.field1 != v.sname2.out.field2) { \ - printf("(%s) %s/%s [%s] != %s/%s [%s]\n", \ - __location__, \ + torture_result(tctx,\ + TORTURE_FAIL,\ + "(%s) %s/%s [%s] != %s/%s [%s]\n", \ + __location__, \ #sname1, #field1, nt_time_string(tctx, s->sname1.field1), \ #sname2, #field2, nt_time_string(tctx, v.sname2.out.field2)); \ ret = false; \ @@ -436,8 +450,10 @@ static bool test_one_file(struct torture_context *tctx, s = find(name); \ if (s) { \ if (!s->sname1.field1 || strcmp(s->sname1.field1, v.sname2.out.field2.s)) { \ - printf("(%s) %s/%s [%s] != %s/%s [%s]\n", \ - __location__, \ + torture_result(tctx,\ + TORTURE_FAIL,\ + "(%s) %s/%s [%s] != %s/%s [%s]\n", \ + __location__, \ #sname1, #field1, s->sname1.field1, \ #sname2, #field2, v.sname2.out.field2.s); \ ret = false; \ @@ -450,8 +466,10 @@ static bool test_one_file(struct torture_context *tctx, if (!s->sname1.field1.s || \ strcmp(s->sname1.field1.s, v.sname2.out.field2.s) || \ wire_bad_flags(&s->sname1.field1, flags, cli->transport)) { \ - printf("(%s) %s/%s [%s] != %s/%s [%s]\n", \ - __location__, \ + torture_result(tctx,\ + TORTURE_FAIL,\ + "(%s) %s/%s [%s] != %s/%s [%s]\n", \ + __location__, \ #sname1, #field1, s->sname1.field1.s, \ #sname2, #field2, v.sname2.out.field2.s); \ ret = false; \ @@ -464,8 +482,10 @@ static bool test_one_file(struct torture_context *tctx, if (!s->sname1.field1.s || \ strcmp(s->sname1.field1.s, fname) || \ wire_bad_flags(&s->sname1.field1, flags, cli->transport)) { \ - printf("(%s) %s/%s [%s] != %s\n", \ - __location__, \ + torture_result(tctx,\ + TORTURE_FAIL,\ + "(%s) %s/%s [%s] != %s\n", \ + __location__, \ #sname1, #field1, s->sname1.field1.s, \ fname); \ ret = false; \ @@ -477,8 +497,10 @@ static bool test_one_file(struct torture_context *tctx, if (s) { \ if (!s->sname1.field1 || \ strcmp(s->sname1.field1, fname)) { \ - printf("(%s) %s/%s [%s] != %s\n", \ - __location__, \ + torture_result(tctx,\ + TORTURE_FAIL,\ + "(%s) %s/%s [%s] != %s\n", \ + __location__, \ #sname1, #field1, s->sname1.field1, \ fname); \ ret = false; \ -- 2.32.0 From 431dce21eb09916349b183709b363b5207e0c54e Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 19 Nov 2021 14:18:47 -0800 Subject: [PATCH 73/92] CVE-2021-44141: s4: torture: In raw.search:test_one_file() remove the leading '\\' in the test filenames. We'll soon be using this under SMB1+POSIX and neither Windows or POSIX need a leading '\\' (and SMB1+POSIX sees the '\\' as part of the name). BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source4/torture/raw/search.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/source4/torture/raw/search.c b/source4/torture/raw/search.c index 07285893f40..9b140928689 100644 --- a/source4/torture/raw/search.c +++ b/source4/torture/raw/search.c @@ -305,8 +305,8 @@ static bool test_one_file(struct torture_context *tctx, { bool ret = true; int fnum; - const char *fname = "\\torture_search.txt"; - const char *fname2 = "\\torture_search-NOTEXIST.txt"; + const char *fname = "torture_search.txt"; + const char *fname2 = "torture_search-NOTEXIST.txt"; NTSTATUS status; int i; union smb_fileinfo all_info, alt_info, name_info, internal_info; @@ -581,20 +581,20 @@ static bool test_one_file(struct torture_context *tctx, short_name, alt_info, alt_name_info, fname, STR_UNICODE); } - CHECK_NAME("STANDARD", standard, name, fname+1, 0); - CHECK_NAME("EA_SIZE", ea_size, name, fname+1, 0); - CHECK_NAME("DIRECTORY_INFO", directory_info, name, fname+1, STR_TERMINATE_ASCII); - CHECK_NAME("FULL_DIRECTORY_INFO", full_directory_info, name, fname+1, STR_TERMINATE_ASCII); + CHECK_NAME("STANDARD", standard, name, fname, 0); + CHECK_NAME("EA_SIZE", ea_size, name, fname, 0); + CHECK_NAME("DIRECTORY_INFO", directory_info, name, fname, STR_TERMINATE_ASCII); + CHECK_NAME("FULL_DIRECTORY_INFO", full_directory_info, name, fname, STR_TERMINATE_ASCII); if (name_info_supported) { - CHECK_NAME("NAME_INFO", name_info, name, fname+1, + CHECK_NAME("NAME_INFO", name_info, name, fname, STR_TERMINATE_ASCII); } - CHECK_NAME("BOTH_DIRECTORY_INFO", both_directory_info, name, fname+1, STR_TERMINATE_ASCII); - CHECK_NAME("ID_FULL_DIRECTORY_INFO", id_full_directory_info, name, fname+1, STR_TERMINATE_ASCII); - CHECK_NAME("ID_BOTH_DIRECTORY_INFO", id_both_directory_info, name, fname+1, STR_TERMINATE_ASCII); - CHECK_UNIX_NAME("UNIX_INFO", unix_info, name, fname+1, STR_TERMINATE_ASCII); + CHECK_NAME("BOTH_DIRECTORY_INFO", both_directory_info, name, fname, STR_TERMINATE_ASCII); + CHECK_NAME("ID_FULL_DIRECTORY_INFO", id_full_directory_info, name, fname, STR_TERMINATE_ASCII); + CHECK_NAME("ID_BOTH_DIRECTORY_INFO", id_both_directory_info, name, fname, STR_TERMINATE_ASCII); + CHECK_UNIX_NAME("UNIX_INFO", unix_info, name, fname, STR_TERMINATE_ASCII); if (internal_info_supported) { CHECK_VAL("ID_FULL_DIRECTORY_INFO", id_full_directory_info, -- 2.32.0 From 4ddad6ef1cb9226fc8a81af8a6a7baf38b4f9938 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 18 Nov 2021 11:48:42 -0800 Subject: [PATCH 74/92] CVE-2021-44141: s3: smbd: Tighten up info level checks for SMB1+POSIX to make sure POSIX was negotiated first. Add knownfail file knownfail.d/posix_infolevel_fails for tests that don't currently negotiate SMB1+POSIX before using SMB1+POSIX calls. These are: samba3.smbtorture_s3.plain.POSIX-BLOCKING-LOCK.smbtorture\(nt4_dc_smb1\) samba3.blackbox.acl_xattr.NT1.nt_affects_posix.* samba3.blackbox.acl_xattr.NT1.nt_affects_chown.* samba3.blackbox.acl_xattr.NT1.nt_affects_chgrp.* samba3.blackbox.inherit_owner.*.NT1.*verify.*unix\ owner.* samba3.unix.info2.info2\(nt4_dc_smb1\) samba3.unix.info2.info2\(ad_dc_smb1\) samba3.raw.search.one\ file\ search.* BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/posix_infolevel_fails | 8 +++ source3/smbd/trans2.c | 60 +++++++++++++++++++--- 2 files changed, 62 insertions(+), 6 deletions(-) create mode 100644 selftest/knownfail.d/posix_infolevel_fails diff --git a/selftest/knownfail.d/posix_infolevel_fails b/selftest/knownfail.d/posix_infolevel_fails new file mode 100644 index 00000000000..78a6781684c --- /dev/null +++ b/selftest/knownfail.d/posix_infolevel_fails @@ -0,0 +1,8 @@ +^samba3.smbtorture_s3.plain.POSIX-BLOCKING-LOCK.smbtorture\(nt4_dc_smb1\) +^samba3.blackbox.acl_xattr.NT1.nt_affects_posix.* +^samba3.blackbox.acl_xattr.NT1.nt_affects_chown.* +^samba3.blackbox.acl_xattr.NT1.nt_affects_chgrp.* +^samba3.blackbox.inherit_owner.*.NT1.*verify.*unix\ owner.* +^samba3.unix.info2.info2\(nt4_dc_smb1\) +^samba3.unix.info2.info2\(ad_dc_smb1\) +^samba3.raw.search.one\ file\ search.* diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c index 0de070c5d08..6d540a2797b 100644 --- a/source3/smbd/trans2.c +++ b/source3/smbd/trans2.c @@ -2715,6 +2715,10 @@ close_if_end = %d requires_resume_key = %d backup_priv = %d level = 0x%x, max_da reply_nterror(req, NT_STATUS_INVALID_LEVEL); goto out; } + if (!req->posix_pathnames) { + reply_nterror(req, NT_STATUS_INVALID_LEVEL); + goto out; + } break; default: reply_nterror(req, NT_STATUS_INVALID_LEVEL); @@ -3181,6 +3185,10 @@ resume_key = %d resume name = %s continue=%d level = %d\n", reply_nterror(req, NT_STATUS_INVALID_LEVEL); return; } + if (!req->posix_pathnames) { + reply_nterror(req, NT_STATUS_INVALID_LEVEL); + return; + } break; default: reply_nterror(req, NT_STATUS_INVALID_LEVEL); @@ -5142,8 +5150,13 @@ NTSTATUS smbd_do_qfilepathinfo(connection_struct *conn, uint32_t access_mask = 0; size_t len = 0; - if (INFO_LEVEL_IS_UNIX(info_level) && !lp_unix_extensions()) { - return NT_STATUS_INVALID_LEVEL; + if (INFO_LEVEL_IS_UNIX(info_level)) { + if (!lp_unix_extensions()) { + return NT_STATUS_INVALID_LEVEL; + } + if (!req->posix_pathnames) { + return NT_STATUS_INVALID_LEVEL; + } } DEBUG(5,("smbd_do_qfilepathinfo: %s (%s) level=%d max_data=%u\n", @@ -5956,9 +5969,15 @@ static void call_trans2qfilepathinfo(connection_struct *conn, DEBUG(3,("call_trans2qfilepathinfo: TRANSACT2_QFILEINFO: level = %d\n", info_level)); - if (INFO_LEVEL_IS_UNIX(info_level) && !lp_unix_extensions()) { - reply_nterror(req, NT_STATUS_INVALID_LEVEL); - return; + if (INFO_LEVEL_IS_UNIX(info_level)) { + if (!lp_unix_extensions()) { + reply_nterror(req, NT_STATUS_INVALID_LEVEL); + return; + } + if (!req->posix_pathnames) { + reply_nterror(req, NT_STATUS_INVALID_LEVEL); + return; + } } /* Initial check for valid fsp ptr. */ @@ -6051,6 +6070,10 @@ static void call_trans2qfilepathinfo(connection_struct *conn, reply_nterror(req, NT_STATUS_INVALID_LEVEL); return; } + if (!req->posix_pathnames) { + reply_nterror(req, NT_STATUS_INVALID_LEVEL); + return; + } } if (req->posix_pathnames) { @@ -9059,7 +9082,9 @@ NTSTATUS smbd_do_setfilepathinfo(connection_struct *conn, if (!lp_unix_extensions()) { return NT_STATUS_INVALID_LEVEL; } - + if (!req->posix_pathnames) { + return NT_STATUS_INVALID_LEVEL; + } status = smbd_do_posix_setfilepathinfo(conn, req, req, @@ -9280,6 +9305,17 @@ static void call_trans2setfilepathinfo(connection_struct *conn, } info_level = SVAL(params,2); + if (INFO_LEVEL_IS_UNIX(info_level)) { + if (!lp_unix_extensions()) { + reply_nterror(req, NT_STATUS_INVALID_LEVEL); + return; + } + if (!req->posix_pathnames) { + reply_nterror(req, NT_STATUS_INVALID_LEVEL); + return; + } + } + smb_fname = fsp->fsp_name; if (fsp_get_pathref_fd(fsp) == -1) { @@ -9358,6 +9394,18 @@ static void call_trans2setfilepathinfo(connection_struct *conn, } info_level = SVAL(params,0); + + if (INFO_LEVEL_IS_UNIX(info_level)) { + if (!lp_unix_extensions()) { + reply_nterror(req, NT_STATUS_INVALID_LEVEL); + return; + } + if (!req->posix_pathnames) { + reply_nterror(req, NT_STATUS_INVALID_LEVEL); + return; + } + } + if (req->posix_pathnames) { srvstr_get_path_posix(req, params, -- 2.32.0 From f569bc7563b213d7a1fe5f276d693ea54f9ee8f4 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Sat, 20 Nov 2021 20:17:11 -0800 Subject: [PATCH 75/92] CVE-2021-44141: s3: smbclient: Give a message if we try and use any POSIX command without negotiating POSIX first. Ensure we only use a POSIX command if POSIX is set up. Issue the message: Command "posix" must be issued before the "XXXX" command can be used. After the parameter parsing has been done. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source3/client/client.c | 79 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/source3/client/client.c b/source3/client/client.c index a8e11044b39..5ad6ee7b844 100644 --- a/source3/client/client.c +++ b/source3/client/client.c @@ -2839,6 +2839,11 @@ static int cmd_posix_open(void) d_printf("posix_open 0\n"); return 1; } + if (CLI_DIRSEP_CHAR != '/') { + d_printf("Command \"posix\" must be issued before " + "the \"posix_open\" command can be used.\n"); + return 1; + } mode = (mode_t)strtol(buf, (char **)NULL, 8); status = cli_resolve_path(ctx, "", @@ -2900,6 +2905,11 @@ static int cmd_posix_mkdir(void) d_printf("posix_mkdir 0\n"); return 1; } + if (CLI_DIRSEP_CHAR != '/') { + d_printf("Command \"posix\" must be issued before " + "the \"posix_mkdir\" command can be used.\n"); + return 1; + } mode = (mode_t)strtol(buf, (char **)NULL, 8); status = cli_resolve_path(ctx, "", @@ -2934,6 +2944,11 @@ static int cmd_posix_unlink(void) d_printf("posix_unlink \n"); return 1; } + if (CLI_DIRSEP_CHAR != '/') { + d_printf("Command \"posix\" must be issued before " + "the \"posix_unlink\" command can be used.\n"); + return 1; + } mask = talloc_asprintf(ctx, "%s%s", client_get_cur_dir(), @@ -2979,6 +2994,11 @@ static int cmd_posix_rmdir(void) d_printf("posix_rmdir \n"); return 1; } + if (CLI_DIRSEP_CHAR != '/') { + d_printf("Command \"posix\" must be issued before " + "the \"posix_rmdir\" command can be used.\n"); + return 1; + } mask = talloc_asprintf(ctx, "%s%s", client_get_cur_dir(), @@ -3178,6 +3198,12 @@ static int cmd_lock(void) return 1; } + if (CLI_DIRSEP_CHAR != '/') { + d_printf("Command \"posix\" must be issued before " + "the \"lock\" command can be used.\n"); + return 1; + } + len = (uint64_t)strtol(buf, (char **)NULL, 16); status = cli_posix_lock(cli, fnum, start, len, true, lock_type); @@ -3214,6 +3240,12 @@ static int cmd_unlock(void) return 1; } + if (CLI_DIRSEP_CHAR != '/') { + d_printf("Command \"posix\" must be issued before " + "the \"unlock\" command can be used.\n"); + return 1; + } + len = (uint64_t)strtol(buf, (char **)NULL, 16); status = cli_posix_unlock(cli, fnum, start, len); @@ -3237,6 +3269,12 @@ static int cmd_posix_whoami(void) bool guest = false; uint32_t i; + if (CLI_DIRSEP_CHAR != '/') { + d_printf("Command \"posix\" must be issued before " + "the \"posix_whoami\" command can be used.\n"); + return 1; + } + status = cli_posix_whoami(cli, ctx, &uid, @@ -3374,6 +3412,12 @@ static int cmd_link(void) return 1; } + if (CLI_DIRSEP_CHAR != '/') { + d_printf("Command \"posix\" must be issued before " + "the \"link\" command can be used.\n"); + return 1; + } + status = cli_posix_hardlink(targetcli, targetname, newname); if (!NT_STATUS_IS_OK(status)) { d_printf("%s linking files (%s -> %s)\n", @@ -3427,6 +3471,12 @@ static int cmd_readlink(void) return 1; } + if (CLI_DIRSEP_CHAR != '/') { + d_printf("Command \"posix\" must be issued before " + "the \"readlink\" command can be used.\n"); + return 1; + } + status = cli_posix_readlink(targetcli, name, talloc_tos(), &linkname); if (!NT_STATUS_IS_OK(status)) { d_printf("%s readlink on file %s\n", @@ -3466,6 +3516,11 @@ static int cmd_symlink(void) link_target = buf; if (SERVER_HAS_UNIX_CIFS(cli)) { + if (CLI_DIRSEP_CHAR != '/') { + d_printf("Command \"posix\" must be issued before " + "the \"symlink\" command can be used.\n"); + return 1; + } newname = talloc_asprintf(ctx, "%s%s", client_get_cur_dir(), buf2); if (!newname) { @@ -3549,6 +3604,12 @@ static int cmd_chmod(void) return 1; } + if (CLI_DIRSEP_CHAR != '/') { + d_printf("Command \"posix\" must be issued before " + "the \"chmod\" command can be used.\n"); + return 1; + } + status = cli_posix_chmod(targetcli, targetname, mode); if (!NT_STATUS_IS_OK(status)) { d_printf("%s chmod file %s 0%o\n", @@ -3713,6 +3774,12 @@ static int cmd_getfacl(void) return 1; } + if (CLI_DIRSEP_CHAR != '/') { + d_printf("Command \"posix\" must be issued before " + "the \"getfacl\" command can be used.\n"); + return 1; + } + status = cli_unix_extensions_version(targetcli, &major, &minor, &caplow, &caphigh); if (!NT_STATUS_IS_OK(status)) { @@ -4012,6 +4079,12 @@ static int cmd_stat(void) return 1; } + if (CLI_DIRSEP_CHAR != '/') { + d_printf("Command \"posix\" must be issued before " + "the \"stat\" command can be used.\n"); + return 1; + } + status = cli_posix_stat(targetcli, targetname, &sbuf); if (!NT_STATUS_IS_OK(status)) { d_printf("%s stat file %s\n", @@ -4126,6 +4199,12 @@ static int cmd_chown(void) return 1; } + if (CLI_DIRSEP_CHAR != '/') { + d_printf("Command \"posix\" must be issued before " + "the \"chown\" command can be used.\n"); + return 1; + } + status = cli_posix_chown(targetcli, targetname, uid, gid); if (!NT_STATUS_IS_OK(status)) { d_printf("%s chown file %s uid=%d, gid=%d\n", -- 2.32.0 From 82b146ed839577b6a879c99ce695255c9bf51074 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 19 Nov 2021 14:44:05 -0800 Subject: [PATCH 76/92] CVE-2021-44141: s4: torture: In raw.search:test_one_file() add a second connection. Change from torture_suite_add_1smb_test() to torture_suite_add_2smb_test(). Not yet used. We will need this to do SMB1+POSIX search calls on a connection on which we have negotiated SMB1+POSIX. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source4/torture/raw/search.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/source4/torture/raw/search.c b/source4/torture/raw/search.c index 9b140928689..f6ad569dd4b 100644 --- a/source4/torture/raw/search.c +++ b/source4/torture/raw/search.c @@ -300,8 +300,9 @@ static union smb_search_data *find(const char *name) /* basic testing of all RAW_SEARCH_* calls using a single file */ -static bool test_one_file(struct torture_context *tctx, - struct smbcli_state *cli) +static bool test_one_file(struct torture_context *tctx, + struct smbcli_state *cli, + struct smbcli_state *cli_unix) { bool ret = true; int fnum; @@ -1571,7 +1572,7 @@ struct torture_suite *torture_raw_search(TALLOC_CTX *mem_ctx) { struct torture_suite *suite = torture_suite_create(mem_ctx, "search"); - torture_suite_add_1smb_test(suite, "one file search", test_one_file); + torture_suite_add_2smb_test(suite, "one file search", test_one_file); torture_suite_add_1smb_test(suite, "many files", test_many_files); torture_suite_add_1smb_test(suite, "sorted", test_sorted); torture_suite_add_1smb_test(suite, "modify search", test_modify_search); -- 2.32.0 From 2500d3a53b90d26fb4c8241a03d842da2277ba6a Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 19 Nov 2021 14:48:20 -0800 Subject: [PATCH 77/92] CVE-2021-44141: s4: torture: raw.search: Add setup_smb1_posix(). Call it on the second connection in test_one_file(). Not yet used. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- source4/torture/raw/search.c | 59 ++++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/source4/torture/raw/search.c b/source4/torture/raw/search.c index f6ad569dd4b..3f7de80ad0f 100644 --- a/source4/torture/raw/search.c +++ b/source4/torture/raw/search.c @@ -297,6 +297,55 @@ static union smb_search_data *find(const char *name) return NULL; } +/* + * Negotiate SMB1+POSIX. + */ + +static NTSTATUS setup_smb1_posix(struct torture_context *tctx, + struct smbcli_state *cli_unix) +{ + struct smb_trans2 tp; + uint16_t setup; + uint8_t data[12]; + uint8_t params[4]; + uint32_t cap = cli_unix->transport->negotiate.capabilities; + + if ((cap & CAP_UNIX) == 0) { + /* + * Server doesn't support SMB1+POSIX. + * The caller will skip the UNIX info + * level anyway. + */ + torture_comment(tctx, + "Server doesn't support SMB1+POSIX\n"); + return NT_STATUS_OK; + } + + /* Setup POSIX on this connection. */ + SSVAL(data, 0, CIFS_UNIX_MAJOR_VERSION); + SSVAL(data, 2, CIFS_UNIX_MINOR_VERSION); + SBVAL(data,4,((uint64_t)( + CIFS_UNIX_POSIX_ACLS_CAP| + CIFS_UNIX_POSIX_PATHNAMES_CAP| + CIFS_UNIX_FCNTL_LOCKS_CAP| + CIFS_UNIX_EXTATTR_CAP| + CIFS_UNIX_POSIX_PATH_OPERATIONS_CAP))); + setup = TRANSACT2_SETFSINFO; + tp.in.max_setup = 0; + tp.in.flags = 0; + tp.in.timeout = 0; + tp.in.setup_count = 1; + tp.in.max_param = 0; + tp.in.max_data = 0; + tp.in.setup = &setup; + tp.in.trans_name = NULL; + SSVAL(params, 0, 0); + SSVAL(params, 2, SMB_SET_CIFS_UNIX_INFO); + tp.in.params = data_blob_talloc(tctx, params, 4); + tp.in.data = data_blob_talloc(tctx, data, 12); + return smb_raw_trans2(cli_unix->tree, tctx, &tp); +} + /* basic testing of all RAW_SEARCH_* calls using a single file */ @@ -315,6 +364,16 @@ static bool test_one_file(struct torture_context *tctx, internal_info_supported; union smb_search_data *s; + status = setup_smb1_posix(tctx, cli_unix); + if (!NT_STATUS_IS_OK(status)) { + torture_result(tctx, + TORTURE_FAIL, + __location__"setup_smb1_posix() failed (%s)\n", + nt_errstr(status)); + ret = false; + goto done; + } + fnum = create_complex_file(cli, tctx, fname); if (fnum == -1) { torture_result(tctx, -- 2.32.0 From 8dd8555a00ec509f279a9a35f219ebe640a8f729 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 19 Nov 2021 14:51:39 -0800 Subject: [PATCH 78/92] CVE-2021-44141: s4: torture: Fix raw.search:test_one_file() by using the SMB1+POSIX connection for POSIX info levels. Remove the following entry in knownfail.d/posix_infolevel_fails. ^samba3.raw.search.one\ file\ search.* from knownfail.d/posix_infolevel_fails BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/posix_infolevel_fails | 1 - source4/torture/raw/search.c | 13 +++++++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/selftest/knownfail.d/posix_infolevel_fails b/selftest/knownfail.d/posix_infolevel_fails index 78a6781684c..4ce4a9cec91 100644 --- a/selftest/knownfail.d/posix_infolevel_fails +++ b/selftest/knownfail.d/posix_infolevel_fails @@ -5,4 +5,3 @@ ^samba3.blackbox.inherit_owner.*.NT1.*verify.*unix\ owner.* ^samba3.unix.info2.info2\(nt4_dc_smb1\) ^samba3.unix.info2.info2\(ad_dc_smb1\) -^samba3.raw.search.one\ file\ search.* diff --git a/source4/torture/raw/search.c b/source4/torture/raw/search.c index 3f7de80ad0f..575bbd03fb7 100644 --- a/source4/torture/raw/search.c +++ b/source4/torture/raw/search.c @@ -389,10 +389,19 @@ static bool test_one_file(struct torture_context *tctx, for (i=0;itransport->negotiate.capabilities; + struct smbcli_state *cli_search = cli; torture_comment(tctx, "Testing %s\n", levels[i].name); - levels[i].status = torture_single_search(cli, tctx, fname, + if (levels[i].data_level == RAW_SEARCH_DATA_UNIX_INFO) { + /* + * For an SMB1+POSIX info level, use the cli_unix + * connection. + */ + cli_search = cli_unix; + } + + levels[i].status = torture_single_search(cli_search, tctx, fname, levels[i].level, levels[i].data_level, 0, @@ -416,7 +425,7 @@ static bool test_one_file(struct torture_context *tctx, continue; } - status = torture_single_search(cli, tctx, fname2, + status = torture_single_search(cli_search, tctx, fname2, levels[i].level, levels[i].data_level, 0, -- 2.32.0 From d51d0febeca33a3cb9ef1a6164c57683efb00a4b Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 19 Nov 2021 12:15:06 -0800 Subject: [PATCH 79/92] CVE-2021-44141: s4: torture: Fix unix.info2 test to actually negotiate SMB1+POSIX before using POSIX calls. Cope with the minor difference in wildcard search return when we're actually using SMB1+POSIX on the server (SMB1+POSIX treats all directory search paths as wildcards). Remove the following entries in knownfail.d/posix_infolevel_fails. samba3.unix.info2.info2\(nt4_dc_smb1\) samba3.unix.info2.info2\(ad_dc_smb1\) BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/posix_infolevel_fails | 2 -- source4/torture/unix/unix_info2.c | 42 ++++++++++++++++++++-- 2 files changed, 40 insertions(+), 4 deletions(-) diff --git a/selftest/knownfail.d/posix_infolevel_fails b/selftest/knownfail.d/posix_infolevel_fails index 4ce4a9cec91..3bfff110771 100644 --- a/selftest/knownfail.d/posix_infolevel_fails +++ b/selftest/knownfail.d/posix_infolevel_fails @@ -3,5 +3,3 @@ ^samba3.blackbox.acl_xattr.NT1.nt_affects_chown.* ^samba3.blackbox.acl_xattr.NT1.nt_affects_chgrp.* ^samba3.blackbox.inherit_owner.*.NT1.*verify.*unix\ owner.* -^samba3.unix.info2.info2\(nt4_dc_smb1\) -^samba3.unix.info2.info2\(ad_dc_smb1\) diff --git a/source4/torture/unix/unix_info2.c b/source4/torture/unix/unix_info2.c index 6b275435551..2098b225e7f 100644 --- a/source4/torture/unix/unix_info2.c +++ b/source4/torture/unix/unix_info2.c @@ -19,6 +19,7 @@ #include "includes.h" #include "libcli/libcli.h" +#include "libcli/raw/raw_proto.h" #include "torture/util.h" #include "torture/unix/proto.h" #include "lib/cmdline/cmdline.h" @@ -53,6 +54,10 @@ static struct smbcli_state *connect_to_server(struct torture_context *tctx) const char *share = torture_setting_string(tctx, "share", NULL); struct smbcli_options options; struct smbcli_session_options session_options; + struct smb_trans2 tp; + uint16_t setup; + uint8_t data[12]; + uint8_t params[4]; lpcfg_smbcli_options(tctx->lp_ctx, &options); lpcfg_smbcli_session_options(tctx->lp_ctx, &session_options); @@ -72,6 +77,33 @@ static struct smbcli_state *connect_to_server(struct torture_context *tctx) return NULL; } + /* Setup POSIX on the server. */ + SSVAL(data, 0, CIFS_UNIX_MAJOR_VERSION); + SSVAL(data, 2, CIFS_UNIX_MINOR_VERSION); + SBVAL(data,4,((uint64_t)( + CIFS_UNIX_POSIX_ACLS_CAP| + CIFS_UNIX_POSIX_PATHNAMES_CAP| + CIFS_UNIX_FCNTL_LOCKS_CAP| + CIFS_UNIX_EXTATTR_CAP| + CIFS_UNIX_POSIX_PATH_OPERATIONS_CAP))); + setup = TRANSACT2_SETFSINFO; + tp.in.max_setup = 0; + tp.in.flags = 0; + tp.in.timeout = 0; + tp.in.setup_count = 1; + tp.in.max_param = 0; + tp.in.max_data = 0; + tp.in.setup = &setup; + tp.in.trans_name = NULL; + SSVAL(params, 0, 0); + SSVAL(params, 2, SMB_SET_CIFS_UNIX_INFO); + tp.in.params = data_blob_talloc(tctx, params, 4); + tp.in.data = data_blob_talloc(tctx, data, 12); + + status = smb_raw_trans2(cli->tree, tctx, &tp); + torture_assert_ntstatus_equal(tctx, status, NT_STATUS_OK, + "doing SMB_SET_CIFS_UNIX_INFO"); + return cli; } @@ -245,8 +277,14 @@ static bool find_single_info2(void *mem_ctx, torture_assert_int_equal(torture, search.t2ffirst.out.count, 1, "expected exactly one result"); - torture_assert_int_equal(torture, search.t2ffirst.out.end_of_search, 1, - "expected end_of_search to be true"); + /* + * In smbd directory listings using POSIX extensions + * always treat the search pathname as a wildcard, + * so don't expect end_of_search to be set here. Wildcard + * searches always need a findnext to end the search. + */ + torture_assert_int_equal(torture, search.t2ffirst.out.end_of_search, 0, + "expected end_of_search to be false"); return check_unix_info2(torture, info2); } -- 2.32.0 From 0f36242f77f50f64eeb9841c0ed7e1aec0ded279 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 19 Nov 2021 12:12:36 -0800 Subject: [PATCH 80/92] CVE-2021-44141: s3: tests: Fix the samba3.blackbox.inherit_owner test to actually negotiate SMB1+POSIX before using POSIX calls. Remove the following entry in knownfail.d/posix_infolevel_fails. samba3.blackbox.inherit_owner.*.NT1.*verify.*unix\ owner.* BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/posix_infolevel_fails | 1 - source3/script/tests/test_inherit_owner.sh | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/selftest/knownfail.d/posix_infolevel_fails b/selftest/knownfail.d/posix_infolevel_fails index 3bfff110771..a865a2055b2 100644 --- a/selftest/knownfail.d/posix_infolevel_fails +++ b/selftest/knownfail.d/posix_infolevel_fails @@ -2,4 +2,3 @@ ^samba3.blackbox.acl_xattr.NT1.nt_affects_posix.* ^samba3.blackbox.acl_xattr.NT1.nt_affects_chown.* ^samba3.blackbox.acl_xattr.NT1.nt_affects_chgrp.* -^samba3.blackbox.inherit_owner.*.NT1.*verify.*unix\ owner.* diff --git a/source3/script/tests/test_inherit_owner.sh b/source3/script/tests/test_inherit_owner.sh index 7e1333787aa..9783235883c 100755 --- a/source3/script/tests/test_inherit_owner.sh +++ b/source3/script/tests/test_inherit_owner.sh @@ -79,7 +79,7 @@ unix_owner_id_is() { local fname=$2 local expected_id=$3 local actual_id - actual_id=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "getfacl $fname" 2>/dev/null | sed -rn 's/^# owner: (.*)/\1/p') + actual_id=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "posix; getfacl $fname" 2>/dev/null | sed -rn 's/^# owner: (.*)/\1/p') if ! test "x$actual_id" = "x$expected_id" ; then echo "Actual uid of $share/$fname is [$actual_id] expected [$expected_id]" exit 1 -- 2.32.0 From 9131e6753bb73f6b0272b5670a51d93cb1256fc6 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 19 Nov 2021 00:05:35 -0800 Subject: [PATCH 81/92] CVE-2021-44141: s3: tests: Fix the samba3.blackbox.acl_xattr test to actually negotiate SMB1+POSIX before using POSIX calls. Remove the following entries in knownfail.d/posix_infolevel_fails. samba3.blackbox.acl_xattr.NT1.nt_affects_posix.* samba3.blackbox.acl_xattr.NT1.nt_affects_chown.* samba3.blackbox.acl_xattr.NT1.nt_affects_chgrp.* BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/posix_infolevel_fails | 3 --- source3/script/tests/test_acl_xattr.sh | 12 ++++++------ 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/selftest/knownfail.d/posix_infolevel_fails b/selftest/knownfail.d/posix_infolevel_fails index a865a2055b2..bf8a884cb16 100644 --- a/selftest/knownfail.d/posix_infolevel_fails +++ b/selftest/knownfail.d/posix_infolevel_fails @@ -1,4 +1 @@ ^samba3.smbtorture_s3.plain.POSIX-BLOCKING-LOCK.smbtorture\(nt4_dc_smb1\) -^samba3.blackbox.acl_xattr.NT1.nt_affects_posix.* -^samba3.blackbox.acl_xattr.NT1.nt_affects_chown.* -^samba3.blackbox.acl_xattr.NT1.nt_affects_chgrp.* diff --git a/source3/script/tests/test_acl_xattr.sh b/source3/script/tests/test_acl_xattr.sh index f134ff79c91..8abd7476244 100755 --- a/source3/script/tests/test_acl_xattr.sh +++ b/source3/script/tests/test_acl_xattr.sh @@ -55,9 +55,9 @@ nt_affects_posix() { local b4 local af local fname="$share.$$" - b4=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "getfacl $fname" 2>/dev/null) || exit 1 + b4=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "posix; getfacl $fname" 2>/dev/null) || exit 1 $SMBCACLS //$SERVER/$share $fname -U $USERNAME%$PASSWORD -a "ACL:$SERVER\force_user:ALLOWED/0x0/READ" 2>/dev/null || exit 1 - af=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "getfacl $fname" 2>/dev/null) || exit 1 + af=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "posix; getfacl $fname" 2>/dev/null) || exit 1 echo "before: $b4" echo "after: $af" echo "${b4}" | grep -q "^# owner:" || exit 1 @@ -90,12 +90,12 @@ nt_affects_chown() { #basic sanity... test "$b4_expected != $af_expected" || exit 1 - b4_actual=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "getfacl $fname" 2>/dev/null) || exit 1 + b4_actual=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "posix; getfacl $fname" 2>/dev/null) || exit 1 echo "${b4_actual}" | grep -q "^# owner:" || exit 1 b4_actual=$(echo "$b4_actual" | sed -rn 's/^# owner: (.*)/\1/p') $SMBCACLS //$SERVER/$share $fname -U $USERNAME%$PASSWORD -a "ACL:$SERVER\force_user:ALLOWED/0x0/FULL" || exit 1 $SMBCACLS //$SERVER/$share $fname -U force_user%$PASSWORD -C force_user 2>/dev/null || exit 1 - af_actual=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "getfacl $fname" 2>/dev/null) || exit 1 + af_actual=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "posix; getfacl $fname" 2>/dev/null) || exit 1 echo "${af_actual}" | grep -q "^# owner:" || exit 1 af_actual=$(echo "$af_actual" | sed -rn 's/^# owner: (.*)/\1/p') echo "before: $b4_actual" @@ -124,11 +124,11 @@ nt_affects_chgrp() { #basic sanity... test "$b4_expected" != "$af_expected" || exit 1 - b4_actual=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "getfacl $fname" 2>/dev/null) || exit 1 + b4_actual=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "posix; getfacl $fname" 2>/dev/null) || exit 1 echo "${b4_actual}" | grep -q "^# group:" || exit 1 b4_actual=$(echo "$b4_actual" | sed -rn 's/^# group: (.*)/\1/p') $SMBCACLS //$SERVER/$share $fname -U $USERNAME%$PASSWORD -G domadmins 2>/dev/null || exit 1 - af_actual=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "getfacl $fname" 2>/dev/null) || exit 1 + af_actual=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "posix; getfacl $fname" 2>/dev/null) || exit 1 echo "${af_actual}" | grep -q "^# group:" || exit 1 af_actual=$(echo "$af_actual" | sed -rn 's/^# group: (.*)/\1/p') echo "before: $b4_actual" -- 2.32.0 From 9b549ae3a0c4a14d50f06b50bd5b83c928cf6915 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 18 Nov 2021 12:16:44 -0800 Subject: [PATCH 82/92] CVE-2021-44141: s3: smbtorture3: Fix POSIX-BLOCKING-LOCK to actually negotiate SMB1+POSIX before using POSIX calls. This must be done before doing POSIX calls on a connection. Remove the final entry in knownfail.d/posix_infolevel_fails samba3.smbtorture_s3.plain.POSIX-BLOCKING-LOCK.smbtorture\(nt4_dc_smb1\) And remove the file knownfail.d/posix_infolevel_fails itself. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/posix_infolevel_fails | 1 - source3/torture/torture.c | 5 +++++ 2 files changed, 5 insertions(+), 1 deletion(-) delete mode 100644 selftest/knownfail.d/posix_infolevel_fails diff --git a/selftest/knownfail.d/posix_infolevel_fails b/selftest/knownfail.d/posix_infolevel_fails deleted file mode 100644 index bf8a884cb16..00000000000 --- a/selftest/knownfail.d/posix_infolevel_fails +++ /dev/null @@ -1 +0,0 @@ -^samba3.smbtorture_s3.plain.POSIX-BLOCKING-LOCK.smbtorture\(nt4_dc_smb1\) diff --git a/source3/torture/torture.c b/source3/torture/torture.c index 0d3b01326b1..e3d26ddb261 100644 --- a/source3/torture/torture.c +++ b/source3/torture/torture.c @@ -8929,6 +8929,11 @@ static bool run_posix_blocking_lock(int dummy) return false; } + status = torture_setup_unix_extensions(cli2); + if (!NT_STATUS_IS_OK(status)) { + return false; + } + cli_setatr(cli1, fname, 0, 0); cli_posix_unlink(cli1, fname); -- 2.32.0 From 2208a7fa741afd93228d9e4ac84db6b1ffe15539 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 7 Dec 2021 12:28:54 -0800 Subject: [PATCH 83/92] CVE-2021-44141: s3: torture: Add samba3.blackbox.test_symlink_traversal.SMB2. Add to knownfail.d/symlink_traversal BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/symlink_traversal | 1 + .../tests/test_symlink_traversal_smb2.sh | 263 ++++++++++++++++++ source3/selftest/tests.py | 6 + 3 files changed, 270 insertions(+) create mode 100644 selftest/knownfail.d/symlink_traversal create mode 100755 source3/script/tests/test_symlink_traversal_smb2.sh diff --git a/selftest/knownfail.d/symlink_traversal b/selftest/knownfail.d/symlink_traversal new file mode 100644 index 00000000000..a8ac4bbae1f --- /dev/null +++ b/selftest/knownfail.d/symlink_traversal @@ -0,0 +1 @@ +^samba3.blackbox.test_symlink_traversal.SMB2.symlink_traversal_SMB2\(fileserver\) diff --git a/source3/script/tests/test_symlink_traversal_smb2.sh b/source3/script/tests/test_symlink_traversal_smb2.sh new file mode 100755 index 00000000000..7e1de6dde1a --- /dev/null +++ b/source3/script/tests/test_symlink_traversal_smb2.sh @@ -0,0 +1,263 @@ +#!/bin/sh + +if [ $# -lt 7 ]; then +cat < "$tmpfile" < Date: Tue, 7 Dec 2021 12:32:19 -0800 Subject: [PATCH 84/92] CVE-2021-44141: s3: torture: Add samba3.blackbox.test_symlink_traversal.SMB1. Add to knownfail.d/symlink_traversal. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/symlink_traversal | 1 + .../tests/test_symlink_traversal_smb1.sh | 263 ++++++++++++++++++ source3/selftest/tests.py | 4 + 3 files changed, 268 insertions(+) create mode 100755 source3/script/tests/test_symlink_traversal_smb1.sh diff --git a/selftest/knownfail.d/symlink_traversal b/selftest/knownfail.d/symlink_traversal index a8ac4bbae1f..2a51ff3f91d 100644 --- a/selftest/knownfail.d/symlink_traversal +++ b/selftest/knownfail.d/symlink_traversal @@ -1 +1,2 @@ ^samba3.blackbox.test_symlink_traversal.SMB2.symlink_traversal_SMB2\(fileserver\) +^samba3.blackbox.test_symlink_traversal.SMB1.symlink_traversal_SMB1\(fileserver_smb1_done\) diff --git a/source3/script/tests/test_symlink_traversal_smb1.sh b/source3/script/tests/test_symlink_traversal_smb1.sh new file mode 100755 index 00000000000..1deaaccbb54 --- /dev/null +++ b/source3/script/tests/test_symlink_traversal_smb1.sh @@ -0,0 +1,263 @@ +#!/bin/sh + +if [ $# -lt 7 ]; then +cat < "$tmpfile" < Date: Tue, 7 Dec 2021 12:34:38 -0800 Subject: [PATCH 85/92] CVE-2021-44141: s3: torture: Add samba3.blackbox.test_symlink_traversal.SMB1.posix Add to knownfail.d/symlink_traversal. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/symlink_traversal | 1 + .../test_symlink_traversal_smb1_posix.sh | 270 ++++++++++++++++++ source3/selftest/tests.py | 5 + 3 files changed, 276 insertions(+) create mode 100755 source3/script/tests/test_symlink_traversal_smb1_posix.sh diff --git a/selftest/knownfail.d/symlink_traversal b/selftest/knownfail.d/symlink_traversal index 2a51ff3f91d..25a4da8f250 100644 --- a/selftest/knownfail.d/symlink_traversal +++ b/selftest/knownfail.d/symlink_traversal @@ -1,2 +1,3 @@ ^samba3.blackbox.test_symlink_traversal.SMB2.symlink_traversal_SMB2\(fileserver\) ^samba3.blackbox.test_symlink_traversal.SMB1.symlink_traversal_SMB1\(fileserver_smb1_done\) +^samba3.blackbox.test_symlink_traversal.SMB1.posix.symlink_traversal_SMB1_posix\(fileserver_smb1_done\) diff --git a/source3/script/tests/test_symlink_traversal_smb1_posix.sh b/source3/script/tests/test_symlink_traversal_smb1_posix.sh new file mode 100755 index 00000000000..6241434dcf6 --- /dev/null +++ b/source3/script/tests/test_symlink_traversal_smb1_posix.sh @@ -0,0 +1,270 @@ +#!/bin/sh + +if [ $# -lt 7 ]; then +cat < "$tmpfile" < Date: Tue, 7 Dec 2021 12:56:51 -0800 Subject: [PATCH 86/92] CVE-2021-44141: s3: torture: In test_smbclient_s3, change the error codes expected for test_widelinks() and test_nosymlinks() from ACCESS_DENIED to NT_STATUS_OBJECT_NAME_NOT_FOUND. For SMB1/2/3 (minus posix) we need to treat bad symlinks as though they don't exist. Add to knwownfail.d/symlink_traversal BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/symlink_traversal | 2 ++ selftest/target/Samba3.pm | 2 +- source3/script/tests/test_smbclient_s3.sh | 10 +++++----- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/selftest/knownfail.d/symlink_traversal b/selftest/knownfail.d/symlink_traversal index 25a4da8f250..840ab38b0f9 100644 --- a/selftest/knownfail.d/symlink_traversal +++ b/selftest/knownfail.d/symlink_traversal @@ -1,3 +1,5 @@ ^samba3.blackbox.test_symlink_traversal.SMB2.symlink_traversal_SMB2\(fileserver\) ^samba3.blackbox.test_symlink_traversal.SMB1.symlink_traversal_SMB1\(fileserver_smb1_done\) ^samba3.blackbox.test_symlink_traversal.SMB1.posix.symlink_traversal_SMB1_posix\(fileserver_smb1_done\) +^samba3.blackbox.smbclient_s3.*.Ensure\ widelinks\ are\ restricted\(.*\) +^samba3.blackbox.smbclient_s3.*.follow\ symlinks\ \=\ no\(.*\) diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm index 84903b87d3e..b901fd2677a 100755 --- a/selftest/target/Samba3.pm +++ b/selftest/target/Samba3.pm @@ -2496,7 +2496,7 @@ sub provision($$) create_file_chmod("$widelinks_target", 0666) or return undef; ## - ## This link should get ACCESS_DENIED + ## This link should get an error ## symlink "$widelinks_target", "$widelinks_shrdir/source"; ## diff --git a/source3/script/tests/test_smbclient_s3.sh b/source3/script/tests/test_smbclient_s3.sh index 89a17656159..e250d4dd106 100755 --- a/source3/script/tests/test_smbclient_s3.sh +++ b/source3/script/tests/test_smbclient_s3.sh @@ -1044,12 +1044,12 @@ EOF return 1 fi -# This should fail with NT_STATUS_ACCESS_DENIED - echo "$out" | grep 'NT_STATUS_ACCESS_DENIED' +# This should fail with NT_STATUS_OBJECT_NAME_NOT_FOUND + echo "$out" | grep 'NT_STATUS_OBJECT_NAME_NOT_FOUND' ret=$? if [ $ret != 0 ] ; then echo "$out" - echo "failed - should get NT_STATUS_ACCESS_DENIED listing \\widelinks_share\\source" + echo "failed - should get NT_STATUS_OBJECT_NAME_NOT_FOUND listing \\widelinks_share\\source" return 1 fi } @@ -1168,11 +1168,11 @@ EOF return 1 fi - echo "$out" | grep 'NT_STATUS_ACCESS_DENIED' + echo "$out" | grep 'NT_STATUS_OBJECT_NAME_NOT_FOUND' ret=$? if [ $ret -ne 0 ] ; then echo "$out" - echo "failed - should get NT_STATUS_ACCESS_DENIED getting \\nosymlinks\\source" + echo "failed - should get NT_STATUS_OBJECT_NAME_NOT_FOUND getting \\nosymlinks\\source" return 1 fi -- 2.32.0 From 6263c0f2abc2d19c26812fb152e4a73bf0cc684d Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 7 Dec 2021 17:56:35 -0800 Subject: [PATCH 87/92] CVE-2021-44141: s3: torture: Change expected error return for samba3.smbtorture_s3.plain.POSIX.smbtorture. Trying to open a symlink as a terminal component should return NT_STATUS_OBJECT_NAME_NOT_FOUND, not NT_STATUS_OBJECT_PATH_NOT_FOUND. Mark as knownfail.d/simple_posix_open until we fix the server. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/simple_posix_open | 1 + source3/torture/torture.c | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) create mode 100644 selftest/knownfail.d/simple_posix_open diff --git a/selftest/knownfail.d/simple_posix_open b/selftest/knownfail.d/simple_posix_open new file mode 100644 index 00000000000..5fcbdbdc2c6 --- /dev/null +++ b/selftest/knownfail.d/simple_posix_open @@ -0,0 +1 @@ +^samba3.smbtorture_s3.plain.POSIX.smbtorture\(.*\) diff --git a/source3/torture/torture.c b/source3/torture/torture.c index e3d26ddb261..c5da3836eac 100644 --- a/source3/torture/torture.c +++ b/source3/torture/torture.c @@ -7984,9 +7984,9 @@ static bool run_simple_posix_open_test(int dummy) goto out; } else { if (!check_both_error(__LINE__, status, ERRDOS, ERRbadpath, - NT_STATUS_OBJECT_PATH_NOT_FOUND)) { + NT_STATUS_OBJECT_NAME_NOT_FOUND)) { printf("POSIX open of %s should have failed " - "with NT_STATUS_OBJECT_PATH_NOT_FOUND, " + "with NT_STATUS_OBJECT_NAME_NOT_FOUND, " "failed with %s instead.\n", sname, nt_errstr(status)); goto out; -- 2.32.0 From 623f283a977d8a105393f5e4169ab86c8501b040 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 7 Dec 2021 11:44:09 -0800 Subject: [PATCH 88/92] CVE-2021-44141: s3: smbd: For SMB1+POSIX clients trying to open a symlink, always return NT_STATUS_OBJECT_NAME_NOT_FOUND. Matches the error return from openat_pathref_fsp(). NT_STATUS_OBJECT_PATH_NOT_FOUND is for a bad component in a path, not a bad terminal symlink. Remove knownfail.d/simple_posix_open, we now pass. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/simple_posix_open | 1 - source3/smbd/open.c | 13 ++++++------- 2 files changed, 6 insertions(+), 8 deletions(-) delete mode 100644 selftest/knownfail.d/simple_posix_open diff --git a/selftest/knownfail.d/simple_posix_open b/selftest/knownfail.d/simple_posix_open deleted file mode 100644 index 5fcbdbdc2c6..00000000000 --- a/selftest/knownfail.d/simple_posix_open +++ /dev/null @@ -1 +0,0 @@ -^samba3.smbtorture_s3.plain.POSIX.smbtorture\(.*\) diff --git a/source3/smbd/open.c b/source3/smbd/open.c index 5ed2c035318..5d2e2a1abf2 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -1447,12 +1447,10 @@ static NTSTATUS open_file(files_struct *fsp, * POSIX client that hit a symlink. We don't want to * return NT_STATUS_STOPPED_ON_SYMLINK to avoid handling * this special error code in all callers, so we map - * this to NT_STATUS_OBJECT_PATH_NOT_FOUND. Historically - * the lower level functions returned status code mapped - * from errno by map_nt_error_from_unix() where ELOOP is - * mapped to NT_STATUS_OBJECT_PATH_NOT_FOUND. + * this to NT_STATUS_OBJECT_NAME_NOT_FOUND to match + * openat_pathref_fsp(). */ - status = NT_STATUS_OBJECT_PATH_NOT_FOUND; + status = NT_STATUS_OBJECT_NAME_NOT_FOUND; } if (!NT_STATUS_IS_OK(status)) { DEBUG(3,("Error opening file %s (%s) (local_flags=%d) " @@ -1535,9 +1533,10 @@ static NTSTATUS open_file(files_struct *fsp, { /* * Don't allow stat opens on symlinks directly unless - * it's a POSIX open. + * it's a POSIX open. Match the return code from + * openat_pathref_fsp(). */ - return NT_STATUS_OBJECT_PATH_NOT_FOUND; + return NT_STATUS_OBJECT_NAME_NOT_FOUND; } if (!fsp->fsp_flags.is_pathref) { -- 2.32.0 From cffc69e12eba26741a86b5081a4374db70f129e0 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 7 Dec 2021 14:33:17 -0800 Subject: [PATCH 89/92] CVE-2021-44141: s3: smbd: Inside check_reduced_name() ensure we return the correct error codes when failing symlinks. NT_STATUS_OBJECT_PATH_NOT_FOUND for a path component failure. NT_STATUS_OBJECT_NAME_NOT_FOUND for a terminal component failure. Remove: samba3.blackbox.test_symlink_traversal.SMB1.posix samba3.blackbox.smbclient_s3.*.Ensure\ widelinks\ are\ restricted\(.*\) samba3.blackbox.smbclient_s3.*.follow\ symlinks\ \=\ no\(.*\) in knownfail.d/symlink_traversal as we now pass these. Only one more fix remaining to get rid of knownfail.d/symlink_traversal completely. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/symlink_traversal | 3 --- source3/smbd/vfs.c | 18 ++++++++++++++++-- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/selftest/knownfail.d/symlink_traversal b/selftest/knownfail.d/symlink_traversal index 840ab38b0f9..2a51ff3f91d 100644 --- a/selftest/knownfail.d/symlink_traversal +++ b/selftest/knownfail.d/symlink_traversal @@ -1,5 +1,2 @@ ^samba3.blackbox.test_symlink_traversal.SMB2.symlink_traversal_SMB2\(fileserver\) ^samba3.blackbox.test_symlink_traversal.SMB1.symlink_traversal_SMB1\(fileserver_smb1_done\) -^samba3.blackbox.test_symlink_traversal.SMB1.posix.symlink_traversal_SMB1_posix\(fileserver_smb1_done\) -^samba3.blackbox.smbclient_s3.*.Ensure\ widelinks\ are\ restricted\(.*\) -^samba3.blackbox.smbclient_s3.*.follow\ symlinks\ \=\ no\(.*\) diff --git a/source3/smbd/vfs.c b/source3/smbd/vfs.c index d8b7b1283fb..a6022902c1d 100644 --- a/source3/smbd/vfs.c +++ b/source3/smbd/vfs.c @@ -1146,6 +1146,7 @@ NTSTATUS check_reduced_name(connection_struct *conn, bool allow_symlinks = true; const char *conn_rootdir; size_t rootdir_len; + bool parent_dir_checked = false; DBG_DEBUG("check_reduced_name [%s] [%s]\n", fname, conn->connectpath); @@ -1207,6 +1208,7 @@ NTSTATUS check_reduced_name(connection_struct *conn, if (resolved_name == NULL) { return NT_STATUS_NO_MEMORY; } + parent_dir_checked = true; } else { resolved_name = resolved_fname->base_name; } @@ -1256,7 +1258,13 @@ NTSTATUS check_reduced_name(connection_struct *conn, conn_rootdir, resolved_name); TALLOC_FREE(resolved_fname); - return NT_STATUS_ACCESS_DENIED; + if (parent_dir_checked) { + /* Part of a component path. */ + return NT_STATUS_OBJECT_PATH_NOT_FOUND; + } else { + /* End of a path. */ + return NT_STATUS_OBJECT_NAME_NOT_FOUND; + } } } @@ -1311,7 +1319,13 @@ NTSTATUS check_reduced_name(connection_struct *conn, p); TALLOC_FREE(resolved_fname); TALLOC_FREE(new_fname); - return NT_STATUS_ACCESS_DENIED; + if (parent_dir_checked) { + /* Part of a component path. */ + return NT_STATUS_OBJECT_PATH_NOT_FOUND; + } else { + /* End of a path. */ + return NT_STATUS_OBJECT_NAME_NOT_FOUND; + } } } -- 2.32.0 From 255243d23d43e6fea1e62449c14f84056c22dbd2 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 7 Dec 2021 14:39:42 -0800 Subject: [PATCH 90/92] CVE-2021-44141: s3: smbd: Fix a subtle bug in the error returns from filename_convert(). If filename_convert() fails to convert the path, we never call check_name(). This means we can return an incorrect error code (NT_STATUS_ACCESS_DENIED) if we ran into a symlink that points outside the share to a non-readable directory. We need to make sure in this case we always call check_name(). Remove knownfail.d/symlink_traversal. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/symlink_traversal | 2 -- source3/smbd/filename.c | 36 ++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 2 deletions(-) delete mode 100644 selftest/knownfail.d/symlink_traversal diff --git a/selftest/knownfail.d/symlink_traversal b/selftest/knownfail.d/symlink_traversal deleted file mode 100644 index 2a51ff3f91d..00000000000 --- a/selftest/knownfail.d/symlink_traversal +++ /dev/null @@ -1,2 +0,0 @@ -^samba3.blackbox.test_symlink_traversal.SMB2.symlink_traversal_SMB2\(fileserver\) -^samba3.blackbox.test_symlink_traversal.SMB1.symlink_traversal_SMB1\(fileserver_smb1_done\) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index 0d82085870c..56ebdd9f370 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -30,6 +30,9 @@ #include "smbd/smbd.h" #include "smbd/globals.h" +static NTSTATUS check_name(connection_struct *conn, + const struct smb_filename *smb_fname); + uint32_t ucf_flags_from_smb_request(struct smb_request *req) { uint32_t ucf_flags = 0; @@ -529,6 +532,39 @@ static NTSTATUS unix_convert_step_search_fail(struct uc_state *state) if (errno == EACCES) { if ((state->ucf_flags & UCF_PREP_CREATEFILE) == 0) { + /* + * Could be a symlink pointing to + * a directory outside the share + * to which we don't have access. + * If so, we need to know that here + * so we can return the correct error code. + * check_name() is never called if we + * error out of filename_convert(). + */ + int ret; + NTSTATUS status; + struct smb_filename dname = (struct smb_filename) { + .base_name = state->dirpath, + .twrp = state->smb_fname->twrp, + }; + + /* handle null paths */ + if ((dname.base_name == NULL) || + (dname.base_name[0] == '\0')) { + return NT_STATUS_ACCESS_DENIED; + } + ret = SMB_VFS_LSTAT(state->conn, &dname); + if (ret != 0) { + return NT_STATUS_ACCESS_DENIED; + } + if (!S_ISLNK(dname.st.st_ex_mode)) { + return NT_STATUS_ACCESS_DENIED; + } + status = check_name(state->conn, &dname); + if (!NT_STATUS_IS_OK(status)) { + /* We know this is an intermediate path. */ + return NT_STATUS_OBJECT_PATH_NOT_FOUND; + } return NT_STATUS_ACCESS_DENIED; } else { /* -- 2.32.0 From f63df6b229931d17654ae4a8dce44bd7ce0972b5 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 7 Dec 2021 22:15:46 -0800 Subject: [PATCH 91/92] CVE-2021-44141: s3: torture: Add a test samba3.blackbox.test_symlink_rename.SMB1.posix that shows we still leak target info across a SMB1+POSIX rename. Add a knownfail.d/posix_sylink_rename BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/posix_sylink_rename | 1 + .../tests/test_symlink_rename_smb1_posix.sh | 186 ++++++++++++++++++ source3/selftest/tests.py | 5 + 3 files changed, 192 insertions(+) create mode 100644 selftest/knownfail.d/posix_sylink_rename create mode 100755 source3/script/tests/test_symlink_rename_smb1_posix.sh diff --git a/selftest/knownfail.d/posix_sylink_rename b/selftest/knownfail.d/posix_sylink_rename new file mode 100644 index 00000000000..9c3cc0a41ba --- /dev/null +++ b/selftest/knownfail.d/posix_sylink_rename @@ -0,0 +1 @@ +^samba3.blackbox.test_symlink_rename.SMB1.posix.symlink_rename_SMB1_posix\(fileserver_smb1_done\) diff --git a/source3/script/tests/test_symlink_rename_smb1_posix.sh b/source3/script/tests/test_symlink_rename_smb1_posix.sh new file mode 100755 index 00000000000..7d2e0037b8d --- /dev/null +++ b/source3/script/tests/test_symlink_rename_smb1_posix.sh @@ -0,0 +1,186 @@ +#!/bin/sh + +if [ $# -lt 7 ]; then +cat < "$tmpfile" < Date: Tue, 7 Dec 2021 22:19:29 -0800 Subject: [PATCH 92/92] CVE-2021-44141: s3: smbd: Inside rename_internals_fsp(), we must use vfs_stat() for existence, not SMB_VFS_STAT(). We need to take SMB1+POSIX into account here and do an LSTAT if it's a POSIX name. Remove knownfail.d/posix_sylink_rename BUG: https://bugzilla.samba.org/show_bug.cgi?id=14911 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/posix_sylink_rename | 1 - source3/smbd/reply.c | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) delete mode 100644 selftest/knownfail.d/posix_sylink_rename diff --git a/selftest/knownfail.d/posix_sylink_rename b/selftest/knownfail.d/posix_sylink_rename deleted file mode 100644 index 9c3cc0a41ba..00000000000 --- a/selftest/knownfail.d/posix_sylink_rename +++ /dev/null @@ -1 +0,0 @@ -^samba3.blackbox.test_symlink_rename.SMB1.posix.symlink_rename_SMB1_posix\(fileserver_smb1_done\) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index cc95bfa3af0..69278a1da87 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -7266,7 +7266,7 @@ NTSTATUS rename_internals_fsp(connection_struct *conn, goto out; } - dst_exists = SMB_VFS_STAT(conn, smb_fname_dst) == 0; + dst_exists = vfs_stat(conn, smb_fname_dst) == 0; if(!replace_if_exists && dst_exists) { DEBUG(3, ("rename_internals_fsp: dest exists doing rename " -- 2.32.0