From: Jeremy Allison Date: Thu, 4 Aug 2022 18:32:05 +0000 (-0700) Subject: s3: smbd: Remove ugly SMB1-specific hack to filename_convert_dirfsp() X-Git-Tag: samba-4.17.0rc1~63 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=5075df4575d9250fa53dcef024589084ad56062d;p=thirdparty%2Fsamba.git s3: smbd: Remove ugly SMB1-specific hack to filename_convert_dirfsp() This was added due to the error code check in test_symlink_traversal_smb1_posix.sh. After careful consideration I've realized the error code expected here is incorrect, and not providing any security benefit. We already check that trying to fetch a file/traverse through a symlink that points outside of a share returns NT_STATUS_OBJECT_PATH_NOT_FOUND, and this is enforced in the symlink checks already inside filename_convert_dirfsp(). If a symlink points to a directory within the share for which the user has no permissions (as is tested here), then there's no benefit in mapping the error code from NT_STATUS_ACCESS_DENIED to NT_STATUS_OBJECT_PATH_NOT_FOUND, as we are not providing any extra information about the filesystem state the user cannot already obtain by normal SMB1+POSIX calls. Change the error code expected in this single test from NT_STATUS_OBJECT_PATH_NOT_FOUND to NT_STATUS_ACCESS_DENIED. Signed-off-by: Jeremy Allison Reviewed-by: Volker Lendecke Autobuild-User(master): Volker Lendecke Autobuild-Date(master): Fri Aug 5 10:24:23 UTC 2022 on sn-devel-184 --- diff --git a/source3/script/tests/test_symlink_traversal_smb1_posix.sh b/source3/script/tests/test_symlink_traversal_smb1_posix.sh index 1bde9c4ffa1..1ad1d56f12a 100755 --- a/source3/script/tests/test_symlink_traversal_smb1_posix.sh +++ b/source3/script/tests/test_symlink_traversal_smb1_posix.sh @@ -252,7 +252,7 @@ test_symlink_traversal_SMB1_posix() # Can't 'get' file inside a directory with no perms. smbclient_expect_error "get" "dir_inside_share_noperms/noperm_file_exists" "" "NT_STATUS_ACCESS_DENIED" || return 1 # In SMB1+POSIX you can't traverse through a symlink that points to a noperm directory. - smbclient_expect_error "get" "symlink_dir_inside_share_noperms/noperm_file_exists" "" "NT_STATUS_OBJECT_PATH_NOT_FOUND" || return 1 + smbclient_expect_error "get" "symlink_dir_inside_share_noperms/noperm_file_exists" "" "NT_STATUS_ACCESS_DENIED" || return 1 # But can list the directory with no perms and the symlink to it. smbclient_expect_error "ls" "dir_inside_share_noperms" "" "NT_STATUS_OK" || return 1 smbclient_expect_error "ls" "symlink_dir_inside_share_noperms" "" "NT_STATUS_OK" || return 1 diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index 7380131a274..87abc8be376 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -2817,22 +2817,6 @@ next: &substitute, &unparsed); -#if defined(WITH_SMB1SERVER) - /* - * This isn't 100% correct, but it gets us close enough - * to the old behavior for SMB1+POSIX libsmbclient. If we went through a - * symlink, and we got NT_STATUS_ACCESS_DENIED on the directory - * containing the target, just don't allow the client to see the - * intermediate path. - */ - if (!conn->sconn->using_smb2 && - (ucf_flags & UCF_POSIX_PATHNAMES) && - symlink_redirects > 0 && - NT_STATUS_EQUAL(status, NT_STATUS_ACCESS_DENIED)) { - return NT_STATUS_OBJECT_PATH_NOT_FOUND; - } -#endif - if (!NT_STATUS_EQUAL(status, NT_STATUS_STOPPED_ON_SYMLINK)) { return status; }