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>
Autobuild-User(master): Jule Anger <janger@samba.org>
Autobuild-Date(master): Tue Oct 25 11:27:02 UTC 2022 on sn-devel-184

diff --git a/source3/script/tests/test_symlink_traversal_smb2.sh b/source3/script/tests/test_symlink_traversal_smb2.sh
index 08929f0962f40931f90b8995e76daa97d8f7f6e5..38719cc8eaef8033ece35e1fbc32360ff7f274ee 100755 (executable)
--- 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 79bb7dffe996be2e5e302fae0d7612f1cae6238c..5bfa4b2f1dfeff2198ff660abaa0533c81b08bdc 100644 (file)
--- a/source3/smbd/filename.c
+++ b/source3/smbd/filename.c
@@ -1369,6 +1369,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;
@@ -1441,17 +1442,17 @@ next:
 
        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;