The Samba-Bugzilla – Attachment 17582 Details for
Bug 15207
CVE-2022-3592 [SECURITY] Samba 4.17 wide links check broken
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
CVE-2022-3592 patch for 4.17
CVE-2022-3592-4.17.2.patch (text/plain), 8.93 KB, created by
Ralph Böhme
on 2022-10-19 08:45:50 UTC
(
hide
)
Description:
CVE-2022-3592 patch for 4.17
Filename:
MIME Type:
Creator:
Ralph Böhme
Created:
2022-10-19 08:45:50 UTC
Size:
8.93 KB
patch
obsolete
>From 36fff1ba12176a26678a9efe7bfd8734e694e853 Mon Sep 17 00:00:00 2001 >From: Volker Lendecke <vl@samba.org> >Date: Mon, 17 Oct 2022 18:06:02 +0200 >Subject: [PATCH 1/4] CVE-2022-3592 smbd: No empty path components in > openat_pathref_dirfsp_nosymlink() > >Upper layers must have filtered this, everything else is a bug > >Bug: https://bugzilla.samba.org/show_bug.cgi?id=15207 >Signed-off-by: Volker Lendecke <vl@samba.org> >--- > source3/smbd/files.c | 6 ++++++ > 1 file changed, 6 insertions(+) > >diff --git a/source3/smbd/files.c b/source3/smbd/files.c >index 179c3e11a76b..9b0c902c0d4d 100644 >--- a/source3/smbd/files.c >+++ b/source3/smbd/files.c >@@ -817,6 +817,12 @@ NTSTATUS openat_pathref_dirfsp_nosymlink( > > next = strv_next(path, rel_fname.base_name); > >+ /* >+ * Path sanitizing further up has cleaned or rejected >+ * empty path components. Assert this here. >+ */ >+ SMB_ASSERT(rel_fname.base_name[0] != '\0'); >+ > if (ISDOT(rel_fname.base_name) || ISDOTDOT(rel_fname.base_name)) { > DBG_DEBUG("%s contains a dot\n", path_in); > status = NT_STATUS_OBJECT_NAME_INVALID; >-- >2.37.3 > > >From 83c11d8c6046e0586b24252e2dbe1f4bd932457c Mon Sep 17 00:00:00 2001 >From: Volker Lendecke <vl@samba.org> >Date: Sat, 15 Oct 2022 14:09:55 +0200 >Subject: [PATCH 2/4] CVE-2022-3592 torture3: Show that our symlink traversal > checks are insecure > >This test shows that we don't properly check whether symlink targets >are inside the exported share. Linking to <share-root>a/etc makes us >loop back into filename_convert_dirfsp_nosymlink() with /etc as a >directory name. > >On Linux systems with openat2(RESOLVE_NO_SYMLINKS) we pass "/etc" >directly into that call after some checks for "."/".." as invalid file >name components. "/etc" is okay for openat2(), but this test must also >succeed on systems without RESOLVE_NO_SYMLINKS (sn-devel-184 for >example). On systems without RESOLVE_NO_SYMLINKS split up the path >"/etc" into path components, in this case "" and "etc". So we pass "" >down to openat(), which correctly fails with ENOENT. > >Summary: Only with RESOLVE_NO_SYMLINKS we're hit by bug 15207, and >this test shows by expecting CONNECTION_DISCONNECTED that we violate >the internal assumption of empty path components with an unexpected >symlink target, making it testable on systems with and without >RESOLVE_NO_SYMLINKS. > >Bug: https://bugzilla.samba.org/show_bug.cgi?id=15207 >Signed-off-by: Volker Lendecke <vl@samba.org> >--- > source3/script/tests/test_symlink_traversal_smb2.sh | 4 ++++ > 1 file changed, 4 insertions(+) > >diff --git a/source3/script/tests/test_symlink_traversal_smb2.sh b/source3/script/tests/test_symlink_traversal_smb2.sh >index 971d53442161..5caee329baf3 100755 >--- a/source3/script/tests/test_symlink_traversal_smb2.sh >+++ b/source3/script/tests/test_symlink_traversal_smb2.sh >@@ -144,6 +144,9 @@ chmod 0 "$dir_outside_share_noperms" > ln -s "dir_inside_share_noperms" "symlink_dir_inside_share_noperms" > mkdir "dir_inside_share_noperms/noperm_subdir_exists" > touch "dir_inside_share_noperms/noperm_subdir_exists/noperm_subdir_file_exists" >+ >+ # Symlink pointing out of the share >+ ln -s "$share_test_dir"a"/etc" x > ) > > # >@@ -345,6 +348,7 @@ test_symlink_traversal_SMB2() > smbclient_expect_error "get" "symlink_to_dir_exists/subdir_exists" "" "NT_STATUS_FILE_IS_A_DIRECTORY" || return 1 > smbclient_expect_error "get" "symlink_to_dir_exists/subdir_exists/noexist1" "" "NT_STATUS_OBJECT_NAME_NOT_FOUND" || return 1 > smbclient_expect_error "get" "symlink_to_dir_exists/subdir_exists/noexist1/noexist2" "" "NT_STATUS_OBJECT_PATH_NOT_FOUND" || return 1 >+ smbclient_expect_error "get" "x/passwd" "passwd" "NT_STATUS_CONNECTION_DISCONNECTED" || return 1 > > # > # Test paths within share with no permissions. >-- >2.37.3 > > >From dcf76926e107da16767b30ee743e4736e7e7ea20 Mon Sep 17 00:00:00 2001 >From: Volker Lendecke <vl@samba.org> >Date: Sat, 15 Oct 2022 13:26:48 +0200 >Subject: [PATCH 3/4] CVE-2022-3592 lib: add subdir_of() to > source3/lib/util_path.c > >Bug: https://bugzilla.samba.org/show_bug.cgi?id=15207 >Signed-off-by: Volker Lendecke <vl@samba.org> >--- > source3/lib/util_path.c | 51 +++++++++++++++++++++++++++++++++++++++++ > source3/lib/util_path.h | 4 ++++ > 2 files changed, 55 insertions(+) > >diff --git a/source3/lib/util_path.c b/source3/lib/util_path.c >index 3591589cb8e1..b8dfec40b03e 100644 >--- a/source3/lib/util_path.c >+++ b/source3/lib/util_path.c >@@ -23,6 +23,7 @@ > > #include "replace.h" > #include <talloc.h> >+#include "lib/util/debug.h" > #include "lib/util/samba_util.h" > #include "lib/util_path.h" > >@@ -204,3 +205,53 @@ char *canonicalize_absolute_path(TALLOC_CTX *ctx, const char *pathname_in) > *p++ = '\0'; > return pathname; > } >+ >+/* >+ * Take two absolute paths, figure out if "subdir" is a proper >+ * subdirectory of "parent". Return the component relative to the >+ * "parent" without the potential "/". Take care of "parent" >+ * possibly ending in "/". >+ */ >+bool subdir_of(const char *parent, >+ size_t parent_len, >+ const char *subdir, >+ const char **_relative) >+{ >+ const char *relative = NULL; >+ bool matched; >+ >+ SMB_ASSERT(parent[0] == '/'); >+ SMB_ASSERT(subdir[0] == '/'); >+ >+ if (parent_len == 1) { >+ /* >+ * Everything is below "/" >+ */ >+ *_relative = subdir+1; >+ return true; >+ } >+ >+ if (parent[parent_len-1] == '/') { >+ parent_len -= 1; >+ } >+ >+ matched = (strncmp(subdir, parent, parent_len) == 0); >+ if (!matched) { >+ return false; >+ } >+ >+ relative = &subdir[parent_len]; >+ >+ if (relative[0] == '\0') { >+ *_relative = relative; /* nothing left */ >+ return true; >+ } >+ >+ if (relative[0] == '/') { >+ /* End of parent must match a '/' in subdir. */ >+ *_relative = relative+1; >+ return true; >+ } >+ >+ return false; >+} >diff --git a/source3/lib/util_path.h b/source3/lib/util_path.h >index 3e7d04de5507..0ea508bf5bbc 100644 >--- a/source3/lib/util_path.h >+++ b/source3/lib/util_path.h >@@ -31,5 +31,9 @@ char *lock_path(TALLOC_CTX *mem_ctx, const char *name); > char *state_path(TALLOC_CTX *mem_ctx, const char *name); > char *cache_path(TALLOC_CTX *mem_ctx, const char *name); > char *canonicalize_absolute_path(TALLOC_CTX *ctx, const char *abs_path); >+bool subdir_of(const char *parent, >+ size_t parent_len, >+ const char *subdir, >+ const char **_relative); > > #endif >-- >2.37.3 > > >From a7755f5573a75673c2f652e60c89464443b9619b Mon Sep 17 00:00:00 2001 >From: Volker Lendecke <vl@samba.org> >Date: Sat, 15 Oct 2022 13:37:17 +0200 >Subject: [PATCH 4/4] CVE-2022-3592 smbd: Slightly simplify > filename_convert_dirfsp() > >subdir_of() calculates the share-relative rest for us, don't do the >strlen(connectpath) calculation twice. subdir_of() also checks that >the target properly ends on a directory. With just strncmp a symlink >to x->/aa/etc would qualify as in share /a, so a "get x/passwd" leads to a >pretty unfortunate result. This is the proper fix for bug 15207, so we >need to change the expected error code to OBJECT_PATH_NOT_FOUND > >Bug: https://bugzilla.samba.org/show_bug.cgi?id=15207 >Signed-off-by: Volker Lendecke <vl@samba.org> >--- > source3/script/tests/test_symlink_traversal_smb2.sh | 2 +- > source3/smbd/filename.c | 11 ++++++----- > 2 files changed, 7 insertions(+), 6 deletions(-) > >diff --git a/source3/script/tests/test_symlink_traversal_smb2.sh b/source3/script/tests/test_symlink_traversal_smb2.sh >index 5caee329baf3..1fb9c67420d9 100755 >--- a/source3/script/tests/test_symlink_traversal_smb2.sh >+++ b/source3/script/tests/test_symlink_traversal_smb2.sh >@@ -348,7 +348,7 @@ test_symlink_traversal_SMB2() > smbclient_expect_error "get" "symlink_to_dir_exists/subdir_exists" "" "NT_STATUS_FILE_IS_A_DIRECTORY" || return 1 > smbclient_expect_error "get" "symlink_to_dir_exists/subdir_exists/noexist1" "" "NT_STATUS_OBJECT_NAME_NOT_FOUND" || return 1 > smbclient_expect_error "get" "symlink_to_dir_exists/subdir_exists/noexist1/noexist2" "" "NT_STATUS_OBJECT_PATH_NOT_FOUND" || return 1 >- smbclient_expect_error "get" "x/passwd" "passwd" "NT_STATUS_CONNECTION_DISCONNECTED" || return 1 >+ smbclient_expect_error "get" "x/passwd" "passwd" "NT_STATUS_OBJECT_PATH_NOT_FOUND" || return 1 > > # > # Test paths within share with no permissions. >diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c >index 0be8e320ffa0..e7873eb124f0 100644 >--- a/source3/smbd/filename.c >+++ b/source3/smbd/filename.c >@@ -1440,6 +1440,7 @@ NTSTATUS filename_convert_dirfsp( > struct smb_filename **_smb_fname) > { > char *substitute = NULL; >+ const char *relative = NULL; > size_t unparsed = 0; > NTSTATUS status; > char *target = NULL; >@@ -1512,17 +1513,17 @@ NTSTATUS filename_convert_dirfsp( > > DBG_DEBUG("abs_target_canon=%s\n", abs_target_canon); > >- in_share = strncmp( >- abs_target_canon, >+ in_share = subdir_of( > conn->connectpath, >- strlen(conn->connectpath)) == 0; >+ strlen(conn->connectpath), >+ abs_target_canon, >+ &relative); > if (!in_share) { > DBG_DEBUG("wide link to %s\n", abs_target_canon); > return NT_STATUS_OBJECT_PATH_NOT_FOUND; > } > >- name_in = talloc_strdup( >- mem_ctx, abs_target_canon + strlen(conn->connectpath) + 1); >+ name_in = talloc_strdup(mem_ctx, relative); > > symlink_redirects += 1; > >-- >2.37.3 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Raw
Flags:
vl
:
review+
slow
:
ci-passed+
Actions:
View
Attachments on
bug 15207
:
17566
|
17575
|
17576
|
17578
|
17579
|
17580
|
17581
| 17582 |
17584