From ba0f63bbadafd6e761d46b6fc2a452989232232e Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Sun, 19 Mar 2017 15:58:17 +0100 Subject: [PATCH 01/13] CVE-2017-2619: s3/smbd: re-open directory after dptr_CloseDir() dptr_CloseDir() will close and invalidate the fsp's file descriptor, we have to reopen it. Bug: https://bugzilla.samba.org/show_bug.cgi?id=12496 Signed-off-by: Ralph Boehme Reviewed-by: Uri Simchoni --- source3/smbd/smb2_query_directory.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/source3/smbd/smb2_query_directory.c b/source3/smbd/smb2_query_directory.c index e18a279..2af029b 100644 --- a/source3/smbd/smb2_query_directory.c +++ b/source3/smbd/smb2_query_directory.c @@ -24,6 +24,7 @@ #include "../libcli/smb/smb_common.h" #include "trans2.h" #include "../lib/util/tevent_ntstatus.h" +#include "system/filesys.h" static struct tevent_req *smbd_smb2_query_directory_send(TALLOC_CTX *mem_ctx, struct tevent_context *ev, @@ -322,7 +323,23 @@ static struct tevent_req *smbd_smb2_query_directory_send(TALLOC_CTX *mem_ctx, } if (in_flags & SMB2_CONTINUE_FLAG_REOPEN) { + int flags; + dptr_CloseDir(fsp); + + /* + * dptr_CloseDir() will close and invalidate the fsp's file + * descriptor, we have to reopen it. + */ + + flags = O_RDONLY; +#ifdef O_DIRECTORY + flags |= O_DIRECTORY; +#endif + status = fd_open(conn, fsp, flags, 0); + if (tevent_req_nterror(req, status)) { + return tevent_req_post(req, ev); + } } if (!smbreq->posix_pathnames) { -- 2.9.3 From 611ed6cc85cc6e0882c6a434a2829743a4fbaecc Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Sun, 19 Mar 2017 18:52:10 +0100 Subject: [PATCH 02/13] CVE-2017-2619: s4/torture: add SMB2_FIND tests with SMB2_CONTINUE_FLAG_REOPEN flag Bug: https://bugzilla.samba.org/show_bug.cgi?id=12496 Signed-off-by: Ralph Boehme Reviewed-by: Uri Simchoni --- source4/torture/smb2/dir.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/source4/torture/smb2/dir.c b/source4/torture/smb2/dir.c index 2e5db91..db906db 100644 --- a/source4/torture/smb2/dir.c +++ b/source4/torture/smb2/dir.c @@ -675,7 +675,7 @@ bool fill_result(void *private_data, return true; } -enum continue_type {CONT_SINGLE, CONT_INDEX, CONT_RESTART}; +enum continue_type {CONT_SINGLE, CONT_INDEX, CONT_RESTART, CONT_REOPEN}; static NTSTATUS multiple_smb2_search(struct smb2_tree *tree, TALLOC_CTX *tctx, @@ -701,6 +701,9 @@ static NTSTATUS multiple_smb2_search(struct smb2_tree *tree, /* The search should start from the beginning everytime */ f.in.continue_flags = SMB2_CONTINUE_FLAG_RESTART; + if (cont_type == CONT_REOPEN) { + f.in.continue_flags = SMB2_CONTINUE_FLAG_REOPEN; + } do { status = smb2_find_level(tree, tree, &f, &count, &d); @@ -804,18 +807,23 @@ static bool test_many_files(struct torture_context *tctx, {"SMB2_FIND_BOTH_DIRECTORY_INFO", "SINGLE", SMB2_FIND_BOTH_DIRECTORY_INFO, RAW_SEARCH_DATA_BOTH_DIRECTORY_INFO, CONT_SINGLE}, {"SMB2_FIND_BOTH_DIRECTORY_INFO", "INDEX", SMB2_FIND_BOTH_DIRECTORY_INFO, RAW_SEARCH_DATA_BOTH_DIRECTORY_INFO, CONT_INDEX}, {"SMB2_FIND_BOTH_DIRECTORY_INFO", "RESTART", SMB2_FIND_BOTH_DIRECTORY_INFO, RAW_SEARCH_DATA_BOTH_DIRECTORY_INFO, CONT_RESTART}, + {"SMB2_FIND_BOTH_DIRECTORY_INFO", "REOPEN", SMB2_FIND_BOTH_DIRECTORY_INFO, RAW_SEARCH_DATA_BOTH_DIRECTORY_INFO, CONT_REOPEN}, {"SMB2_FIND_DIRECTORY_INFO", "SINGLE", SMB2_FIND_DIRECTORY_INFO, RAW_SEARCH_DATA_DIRECTORY_INFO, CONT_SINGLE}, {"SMB2_FIND_DIRECTORY_INFO", "INDEX", SMB2_FIND_DIRECTORY_INFO, RAW_SEARCH_DATA_DIRECTORY_INFO, CONT_INDEX}, {"SMB2_FIND_DIRECTORY_INFO", "RESTART", SMB2_FIND_DIRECTORY_INFO, RAW_SEARCH_DATA_DIRECTORY_INFO, CONT_RESTART}, + {"SMB2_FIND_DIRECTORY_INFO", "REOPEN", SMB2_FIND_DIRECTORY_INFO, RAW_SEARCH_DATA_DIRECTORY_INFO, CONT_REOPEN}, {"SMB2_FIND_FULL_DIRECTORY_INFO", "SINGLE", SMB2_FIND_FULL_DIRECTORY_INFO, RAW_SEARCH_DATA_FULL_DIRECTORY_INFO, CONT_SINGLE}, {"SMB2_FIND_FULL_DIRECTORY_INFO", "INDEX", SMB2_FIND_FULL_DIRECTORY_INFO, RAW_SEARCH_DATA_FULL_DIRECTORY_INFO, CONT_INDEX}, {"SMB2_FIND_FULL_DIRECTORY_INFO", "RESTART", SMB2_FIND_FULL_DIRECTORY_INFO, RAW_SEARCH_DATA_FULL_DIRECTORY_INFO, CONT_RESTART}, + {"SMB2_FIND_FULL_DIRECTORY_INFO", "REOPEN", SMB2_FIND_FULL_DIRECTORY_INFO, RAW_SEARCH_DATA_FULL_DIRECTORY_INFO, CONT_REOPEN}, {"SMB2_FIND_ID_FULL_DIRECTORY_INFO", "SINGLE", SMB2_FIND_ID_FULL_DIRECTORY_INFO, RAW_SEARCH_DATA_ID_FULL_DIRECTORY_INFO, CONT_SINGLE}, {"SMB2_FIND_ID_FULL_DIRECTORY_INFO", "INDEX", SMB2_FIND_ID_FULL_DIRECTORY_INFO, RAW_SEARCH_DATA_ID_FULL_DIRECTORY_INFO, CONT_INDEX}, {"SMB2_FIND_ID_FULL_DIRECTORY_INFO", "RESTART", SMB2_FIND_ID_FULL_DIRECTORY_INFO, RAW_SEARCH_DATA_ID_FULL_DIRECTORY_INFO, CONT_RESTART}, + {"SMB2_FIND_ID_FULL_DIRECTORY_INFO", "REOPEN", SMB2_FIND_ID_FULL_DIRECTORY_INFO, RAW_SEARCH_DATA_ID_FULL_DIRECTORY_INFO, CONT_REOPEN}, {"SMB2_FIND_ID_BOTH_DIRECTORY_INFO", "SINGLE", SMB2_FIND_ID_BOTH_DIRECTORY_INFO, RAW_SEARCH_DATA_ID_BOTH_DIRECTORY_INFO, CONT_SINGLE}, {"SMB2_FIND_ID_BOTH_DIRECTORY_INFO", "INDEX", SMB2_FIND_ID_BOTH_DIRECTORY_INFO, RAW_SEARCH_DATA_ID_BOTH_DIRECTORY_INFO, CONT_INDEX}, - {"SMB2_FIND_ID_BOTH_DIRECTORY_INFO", "RESTART", SMB2_FIND_ID_BOTH_DIRECTORY_INFO, RAW_SEARCH_DATA_ID_BOTH_DIRECTORY_INFO, CONT_RESTART} + {"SMB2_FIND_ID_BOTH_DIRECTORY_INFO", "RESTART", SMB2_FIND_ID_BOTH_DIRECTORY_INFO, RAW_SEARCH_DATA_ID_BOTH_DIRECTORY_INFO, CONT_RESTART}, + {"SMB2_FIND_ID_BOTH_DIRECTORY_INFO", "REOPEN", SMB2_FIND_ID_BOTH_DIRECTORY_INFO, RAW_SEARCH_DATA_ID_BOTH_DIRECTORY_INFO, CONT_REOPEN}, }; smb2_deltree(tree, DNAME); -- 2.9.3 From adf5ede2cc51a181d168458971f405ad5e4fa964 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Mon, 19 Dec 2016 11:55:56 -0800 Subject: [PATCH 03/13] CVE-2017-2619: s3: smbd: Create wrapper function for OpenDir in preparation for making robust. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12496 Signed-off-by: Jeremy Allison Reviewed-by: Uri Simchoni --- source3/smbd/dir.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/source3/smbd/dir.c b/source3/smbd/dir.c index 3c6f000..b22d92d 100644 --- a/source3/smbd/dir.c +++ b/source3/smbd/dir.c @@ -1630,7 +1630,8 @@ static int smb_Dir_destructor(struct smb_Dir *dirp) Open a directory. ********************************************************************/ -struct smb_Dir *OpenDir(TALLOC_CTX *mem_ctx, connection_struct *conn, +static struct smb_Dir *OpenDir_internal(TALLOC_CTX *mem_ctx, + connection_struct *conn, const struct smb_filename *smb_dname, const char *mask, uint32_t attr) @@ -1672,6 +1673,18 @@ struct smb_Dir *OpenDir(TALLOC_CTX *mem_ctx, connection_struct *conn, return NULL; } +struct smb_Dir *OpenDir(TALLOC_CTX *mem_ctx, connection_struct *conn, + const struct smb_filename *smb_dname, + const char *mask, + uint32_t attr) +{ + return OpenDir_internal(mem_ctx, + conn, + smb_dname, + mask, + attr); +} + /******************************************************************* Open a directory from an fsp. ********************************************************************/ -- 2.9.3 From 4e32ea8841110c2d4d18ae6098d42e154d9f1237 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Mon, 19 Dec 2016 16:25:26 -0800 Subject: [PATCH 04/13] CVE-2017-2619: s3: smbd: Opendir_internal() early return if SMB_VFS_OPENDIR failed. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12496 Signed-off-by: Jeremy Allison Reviewed-by: Uri Simchoni --- source3/smbd/dir.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/source3/smbd/dir.c b/source3/smbd/dir.c index b22d92d..a5d172a 100644 --- a/source3/smbd/dir.c +++ b/source3/smbd/dir.c @@ -1643,6 +1643,15 @@ static struct smb_Dir *OpenDir_internal(TALLOC_CTX *mem_ctx, return NULL; } + dirp->dir = SMB_VFS_OPENDIR(conn, smb_dname, mask, attr); + + if (!dirp->dir) { + DEBUG(5,("OpenDir: Can't open %s. %s\n", + smb_dname->base_name, + strerror(errno) )); + goto fail; + } + dirp->conn = conn; dirp->name_cache_size = lp_directory_name_cache_size(SNUM(conn)); @@ -1657,15 +1666,6 @@ static struct smb_Dir *OpenDir_internal(TALLOC_CTX *mem_ctx, } talloc_set_destructor(dirp, smb_Dir_destructor); - dirp->dir = SMB_VFS_OPENDIR(conn, dirp->dir_smb_fname, mask, attr); - - if (!dirp->dir) { - DEBUG(5,("OpenDir: Can't open %s. %s\n", - dirp->dir_smb_fname->base_name, - strerror(errno) )); - goto fail; - } - return dirp; fail: -- 2.9.3 From f9d3cd47ae78507ac6ad90d5a93d23b2b12c694d Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Mon, 19 Dec 2016 16:35:00 -0800 Subject: [PATCH 05/13] CVE-2017-2619: s3: smbd: Create and use open_dir_safely(). Use from OpenDir(). Hardens OpenDir against TOC/TOU races. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12496 Signed-off-by: Jeremy Allison Reviewed-by: Uri Simchoni --- source3/smbd/dir.c | 77 +++++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 70 insertions(+), 7 deletions(-) diff --git a/source3/smbd/dir.c b/source3/smbd/dir.c index a5d172a..2b107a9 100644 --- a/source3/smbd/dir.c +++ b/source3/smbd/dir.c @@ -1655,12 +1655,6 @@ static struct smb_Dir *OpenDir_internal(TALLOC_CTX *mem_ctx, dirp->conn = conn; dirp->name_cache_size = lp_directory_name_cache_size(SNUM(conn)); - dirp->dir_smb_fname = cp_smb_filename(dirp, smb_dname); - if (!dirp->dir_smb_fname) { - errno = ENOMEM; - goto fail; - } - if (sconn && !sconn->using_smb2) { sconn->searches.dirhandles_open++; } @@ -1673,12 +1667,81 @@ static struct smb_Dir *OpenDir_internal(TALLOC_CTX *mem_ctx, return NULL; } +/**************************************************************************** + Open a directory handle by pathname, ensuring it's under the share path. +****************************************************************************/ + +static struct smb_Dir *open_dir_safely(TALLOC_CTX *ctx, + connection_struct *conn, + const struct smb_filename *smb_dname, + const char *wcard, + uint32_t attr) +{ + struct smb_Dir *dir_hnd = NULL; + struct smb_filename *smb_fname_cwd = NULL; + char *saved_dir = vfs_GetWd(ctx, conn); + NTSTATUS status; + + if (saved_dir == NULL) { + return NULL; + } + + if (vfs_ChDir(conn, smb_dname->base_name) == -1) { + goto out; + } + + smb_fname_cwd = synthetic_smb_fname(talloc_tos(), + ".", + NULL, + NULL, + smb_dname->flags); + if (smb_fname_cwd == NULL) { + goto out; + } + + /* + * Now the directory is pinned, use + * REALPATH to ensure we can access it. + */ + status = check_name(conn, "."); + if (!NT_STATUS_IS_OK(status)) { + goto out; + } + + dir_hnd = OpenDir_internal(ctx, + conn, + smb_fname_cwd, + wcard, + attr); + + if (dir_hnd == NULL) { + goto out; + } + + /* + * OpenDir_internal only gets "." as the dir name. + * Store the real dir name here. + */ + + dir_hnd->dir_smb_fname = cp_smb_filename(dir_hnd, smb_dname); + if (!dir_hnd->dir_smb_fname) { + TALLOC_FREE(dir_hnd); + errno = ENOMEM; + } + + out: + + vfs_ChDir(conn, saved_dir); + TALLOC_FREE(saved_dir); + return dir_hnd; +} + struct smb_Dir *OpenDir(TALLOC_CTX *mem_ctx, connection_struct *conn, const struct smb_filename *smb_dname, const char *mask, uint32_t attr) { - return OpenDir_internal(mem_ctx, + return open_dir_safely(mem_ctx, conn, smb_dname, mask, -- 2.9.3 From 081d6931e99a4c2689c87a96b3bab92b5cc5d0e0 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Mon, 19 Dec 2016 12:13:20 -0800 Subject: [PATCH 06/13] CVE-2017-2619: s3: smbd: OpenDir_fsp() use early returns. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12496 Signed-off-by: Jeremy Allison Reviewed-by: Uri Simchoni --- source3/smbd/dir.c | 34 +++++++++++++++++++++------------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/source3/smbd/dir.c b/source3/smbd/dir.c index 2b107a9..12edf80 100644 --- a/source3/smbd/dir.c +++ b/source3/smbd/dir.c @@ -1761,7 +1761,17 @@ static struct smb_Dir *OpenDir_fsp(TALLOC_CTX *mem_ctx, connection_struct *conn, struct smbd_server_connection *sconn = conn->sconn; if (!dirp) { - return NULL; + goto fail; + } + + if (!fsp->is_directory) { + errno = EBADF; + goto fail; + } + + if (fsp->fh->fd == -1) { + errno = EBADF; + goto fail; } dirp->conn = conn; @@ -1778,18 +1788,16 @@ static struct smb_Dir *OpenDir_fsp(TALLOC_CTX *mem_ctx, connection_struct *conn, } talloc_set_destructor(dirp, smb_Dir_destructor); - if (fsp->is_directory && fsp->fh->fd != -1) { - dirp->dir = SMB_VFS_FDOPENDIR(fsp, mask, attr); - if (dirp->dir != NULL) { - dirp->fsp = fsp; - } else { - DEBUG(10,("OpenDir_fsp: SMB_VFS_FDOPENDIR on %s returned " - "NULL (%s)\n", - dirp->dir_smb_fname->base_name, - strerror(errno))); - if (errno != ENOSYS) { - return NULL; - } + dirp->dir = SMB_VFS_FDOPENDIR(fsp, mask, attr); + if (dirp->dir != NULL) { + dirp->fsp = fsp; + } else { + DEBUG(10,("OpenDir_fsp: SMB_VFS_FDOPENDIR on %s returned " + "NULL (%s)\n", + dirp->dir_smb_fname->base_name, + strerror(errno))); + if (errno != ENOSYS) { + return NULL; } } -- 2.9.3 From 5da26303414de3506299b7be1eb87feb4e5003bc Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Mon, 19 Dec 2016 12:15:59 -0800 Subject: [PATCH 07/13] CVE-2017-2619: s3: smbd: OpenDir_fsp() - Fix memory leak on error. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12496 Signed-off-by: Jeremy Allison Reviewed-by: Uri Simchoni --- source3/smbd/dir.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source3/smbd/dir.c b/source3/smbd/dir.c index 12edf80..42e787b 100644 --- a/source3/smbd/dir.c +++ b/source3/smbd/dir.c @@ -1797,7 +1797,7 @@ static struct smb_Dir *OpenDir_fsp(TALLOC_CTX *mem_ctx, connection_struct *conn, dirp->dir_smb_fname->base_name, strerror(errno))); if (errno != ENOSYS) { - return NULL; + goto fail; } } -- 2.9.3 From d2b707a59c7123526657c97724570f0faa4c4505 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Mon, 19 Dec 2016 12:32:07 -0800 Subject: [PATCH 08/13] CVE-2017-2619: s3: smbd: Move the reference counting and destructor setup to just before retuning success. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12496 Signed-off-by: Jeremy Allison Reviewed-by: Uri Simchoni --- source3/smbd/dir.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/source3/smbd/dir.c b/source3/smbd/dir.c index 42e787b..2fd5085 100644 --- a/source3/smbd/dir.c +++ b/source3/smbd/dir.c @@ -1783,11 +1783,6 @@ static struct smb_Dir *OpenDir_fsp(TALLOC_CTX *mem_ctx, connection_struct *conn, goto fail; } - if (sconn && !sconn->using_smb2) { - sconn->searches.dirhandles_open++; - } - talloc_set_destructor(dirp, smb_Dir_destructor); - dirp->dir = SMB_VFS_FDOPENDIR(fsp, mask, attr); if (dirp->dir != NULL) { dirp->fsp = fsp; @@ -1816,6 +1811,11 @@ static struct smb_Dir *OpenDir_fsp(TALLOC_CTX *mem_ctx, connection_struct *conn, goto fail; } + if (sconn && !sconn->using_smb2) { + sconn->searches.dirhandles_open++; + } + talloc_set_destructor(dirp, smb_Dir_destructor); + return dirp; fail: -- 2.9.3 From cb51358ecc9988067ae0eeee72bafb644721db83 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Mon, 19 Dec 2016 12:35:32 -0800 Subject: [PATCH 09/13] CVE-2017-2619: s3: smbd: Correctly fallback to open_dir_safely if FDOPENDIR not supported on system. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12496 Signed-off-by: Jeremy Allison Reviewed-by: Uri Simchoni --- source3/smbd/dir.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/source3/smbd/dir.c b/source3/smbd/dir.c index 2fd5085..1348d12 100644 --- a/source3/smbd/dir.c +++ b/source3/smbd/dir.c @@ -1797,20 +1797,15 @@ static struct smb_Dir *OpenDir_fsp(TALLOC_CTX *mem_ctx, connection_struct *conn, } if (dirp->dir == NULL) { - /* FDOPENDIR didn't work. Use OPENDIR instead. */ - dirp->dir = SMB_VFS_OPENDIR(conn, - dirp->dir_smb_fname, + /* FDOPENDIR is not supported. Use OPENDIR instead. */ + TALLOC_FREE(dirp); + return open_dir_safely(mem_ctx, + conn, + fsp->fsp_name, mask, attr); } - if (!dirp->dir) { - DEBUG(5,("OpenDir_fsp: Can't open %s. %s\n", - dirp->dir_smb_fname->base_name, - strerror(errno) )); - goto fail; - } - if (sconn && !sconn->using_smb2) { sconn->searches.dirhandles_open++; } -- 2.9.3 From e1f1312dbf00475dd72c72b6b93a7b2429f04839 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 15 Dec 2016 12:52:13 -0800 Subject: [PATCH 10/13] CVE-2017-2619: s3: smbd: Remove O_NOFOLLOW guards. We insist on O_NOFOLLOW existing. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12496 Signed-off-by: Jeremy Allison Reviewed-by: Uri Simchoni --- source3/smbd/open.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/source3/smbd/open.c b/source3/smbd/open.c index e0e4705..c562edf 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -366,8 +366,7 @@ NTSTATUS fd_open(struct connection_struct *conn, struct smb_filename *smb_fname = fsp->fsp_name; NTSTATUS status = NT_STATUS_OK; -#ifdef O_NOFOLLOW - /* + /* * Never follow symlinks on a POSIX client. The * client should be doing this. */ @@ -375,12 +374,10 @@ NTSTATUS fd_open(struct connection_struct *conn, if ((fsp->posix_flags & FSP_POSIX_FLAGS_OPEN) || !lp_follow_symlinks(SNUM(conn))) { flags |= O_NOFOLLOW; } -#endif fsp->fh->fd = SMB_VFS_OPEN(conn, smb_fname, fsp, flags, mode); if (fsp->fh->fd == -1) { int posix_errno = errno; -#ifdef O_NOFOLLOW #if defined(ENOTSUP) && defined(OSF1) /* handle special Tru64 errno */ if (errno == ENOTSUP) { @@ -397,7 +394,6 @@ NTSTATUS fd_open(struct connection_struct *conn, if (errno == EMLINK) { posix_errno = ELOOP; } -#endif /* O_NOFOLLOW */ status = map_nt_error_from_unix(posix_errno); if (errno == EMFILE) { static time_t last_warned = 0L; -- 2.9.3 From fa2a82de0771bc073845e8fd2583b27955bd876c Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 15 Dec 2016 12:56:08 -0800 Subject: [PATCH 11/13] CVE-2017-2619: s3: smbd: Move special handling of symlink errno's into a utility function. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12496 Signed-off-by: Jeremy Allison Reviewed-by: Uri Simchoni --- source3/smbd/open.c | 43 ++++++++++++++++++++++++++----------------- 1 file changed, 26 insertions(+), 17 deletions(-) diff --git a/source3/smbd/open.c b/source3/smbd/open.c index c562edf..a6f5cc8 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -355,6 +355,31 @@ static NTSTATUS check_base_file_access(struct connection_struct *conn, } /**************************************************************************** + Handle differing symlink errno's +****************************************************************************/ + +static int link_errno_convert(int err) +{ +#if defined(ENOTSUP) && defined(OSF1) + /* handle special Tru64 errno */ + if (err == ENOTSUP) { + err = ELOOP; + } +#endif /* ENOTSUP */ +#ifdef EFTYPE + /* fix broken NetBSD errno */ + if (err == EFTYPE) { + err = ELOOP; + } +#endif /* EFTYPE */ + /* fix broken FreeBSD errno */ + if (err == EMLINK) { + err = ELOOP; + } + return err; +} + +/**************************************************************************** fd support routines - attempt to do a dos_open. ****************************************************************************/ @@ -377,23 +402,7 @@ NTSTATUS fd_open(struct connection_struct *conn, fsp->fh->fd = SMB_VFS_OPEN(conn, smb_fname, fsp, flags, mode); if (fsp->fh->fd == -1) { - int posix_errno = errno; -#if defined(ENOTSUP) && defined(OSF1) - /* handle special Tru64 errno */ - if (errno == ENOTSUP) { - posix_errno = ELOOP; - } -#endif /* ENOTSUP */ -#ifdef EFTYPE - /* fix broken NetBSD errno */ - if (errno == EFTYPE) { - posix_errno = ELOOP; - } -#endif /* EFTYPE */ - /* fix broken FreeBSD errno */ - if (errno == EMLINK) { - posix_errno = ELOOP; - } + int posix_errno = link_errno_convert(errno); status = map_nt_error_from_unix(posix_errno); if (errno == EMFILE) { static time_t last_warned = 0L; -- 2.9.3 From c32bd4302358794fbc3b83c3aecfd92458de382c Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 15 Dec 2016 13:04:46 -0800 Subject: [PATCH 12/13] CVE-2017-2619: s3: smbd: Add the core functions to prevent symlink open races. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12496 Signed-off-by: Jeremy Allison Reviewed-by: Uri Simchoni --- source3/smbd/open.c | 238 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 238 insertions(+) diff --git a/source3/smbd/open.c b/source3/smbd/open.c index a6f5cc8..4c2ee47 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -379,6 +379,244 @@ static int link_errno_convert(int err) return err; } +static int non_widelink_open(struct connection_struct *conn, + const char *conn_rootdir, + files_struct *fsp, + struct smb_filename *smb_fname, + int flags, + mode_t mode, + unsigned int link_depth); + +/**************************************************************************** + Follow a symlink in userspace. +****************************************************************************/ + +static int process_symlink_open(struct connection_struct *conn, + const char *conn_rootdir, + files_struct *fsp, + struct smb_filename *smb_fname, + int flags, + mode_t mode, + unsigned int link_depth) +{ + int fd = -1; + char *link_target = NULL; + int link_len = -1; + char *oldwd = NULL; + size_t rootdir_len = 0; + char *resolved_name = NULL; + bool matched = false; + int saved_errno = 0; + + /* + * Ensure we don't get stuck in a symlink loop. + */ + link_depth++; + if (link_depth >= 20) { + errno = ELOOP; + goto out; + } + + /* Allocate space for the link target. */ + link_target = talloc_array(talloc_tos(), char, PATH_MAX); + if (link_target == NULL) { + errno = ENOMEM; + goto out; + } + + /* Read the link target. */ + link_len = SMB_VFS_READLINK(conn, + smb_fname->base_name, + link_target, + PATH_MAX - 1); + if (link_len == -1) { + goto out; + } + + /* Ensure it's at least null terminated. */ + link_target[link_len] = '\0'; + + /* Convert to an absolute path. */ + resolved_name = SMB_VFS_REALPATH(conn, link_target); + if (resolved_name == NULL) { + goto out; + } + + /* + * We know conn_rootdir starts with '/' and + * does not end in '/'. FIXME ! Should we + * smb_assert this ? + */ + rootdir_len = strlen(conn_rootdir); + + matched = (strncmp(conn_rootdir, resolved_name, rootdir_len) == 0); + if (!matched) { + errno = EACCES; + goto out; + } + + /* + * Turn into a path relative to the share root. + */ + if (resolved_name[rootdir_len] == '\0') { + /* Link to the root of the share. */ + smb_fname->base_name = talloc_strdup(talloc_tos(), "."); + if (smb_fname->base_name == NULL) { + errno = ENOMEM; + goto out; + } + } else if (resolved_name[rootdir_len] == '/') { + smb_fname->base_name = &resolved_name[rootdir_len+1]; + } else { + errno = EACCES; + goto out; + } + + oldwd = vfs_GetWd(talloc_tos(), conn); + if (oldwd == NULL) { + goto out; + } + + /* Ensure we operate from the root of the share. */ + if (vfs_ChDir(conn, conn_rootdir) == -1) { + goto out; + } + + /* And do it all again.. */ + fd = non_widelink_open(conn, + conn_rootdir, + fsp, + smb_fname, + flags, + mode, + link_depth); + if (fd == -1) { + saved_errno = errno; + } + + out: + + SAFE_FREE(resolved_name); + TALLOC_FREE(link_target); + if (oldwd != NULL) { + int ret = vfs_ChDir(conn, oldwd); + if (ret == -1) { + smb_panic("unable to get back to old directory\n"); + } + TALLOC_FREE(oldwd); + } + if (saved_errno != 0) { + errno = saved_errno; + } + return fd; +} + +/**************************************************************************** + Non-widelink open. +****************************************************************************/ + +static int non_widelink_open(struct connection_struct *conn, + const char *conn_rootdir, + files_struct *fsp, + struct smb_filename *smb_fname, + int flags, + mode_t mode, + unsigned int link_depth) +{ + NTSTATUS status; + int fd = -1; + struct smb_filename *smb_fname_rel = NULL; + int saved_errno = 0; + char *oldwd = NULL; + char *parent_dir = NULL; + const char *final_component = NULL; + + if (!parent_dirname(talloc_tos(), + smb_fname->base_name, + &parent_dir, + &final_component)) { + goto out; + } + + oldwd = vfs_GetWd(talloc_tos(), conn); + if (oldwd == NULL) { + goto out; + } + + /* Pin parent directory in place. */ + if (vfs_ChDir(conn, parent_dir) == -1) { + goto out; + } + + /* Ensure the relative path is below the share. */ + status = check_reduced_name(conn, final_component); + if (!NT_STATUS_IS_OK(status)) { + saved_errno = map_errno_from_nt_status(status); + goto out; + } + + smb_fname_rel = synthetic_smb_fname(talloc_tos(), + final_component, + smb_fname->stream_name, + &smb_fname->st, + smb_fname->flags); + + flags |= O_NOFOLLOW; + + { + struct smb_filename *tmp_name = fsp->fsp_name; + fsp->fsp_name = smb_fname_rel; + fd = SMB_VFS_OPEN(conn, smb_fname_rel, fsp, flags, mode); + fsp->fsp_name = tmp_name; + } + + if (fd == -1) { + saved_errno = link_errno_convert(errno); + if (saved_errno == ELOOP) { + if (fsp->posix_flags & FSP_POSIX_FLAGS_OPEN) { + /* Never follow symlinks on posix open. */ + goto out; + } + if (!lp_follow_symlinks(SNUM(conn))) { + /* Explicitly no symlinks. */ + goto out; + } + /* + * We have a symlink. Follow in userspace + * to ensure it's under the share definition. + */ + fd = process_symlink_open(conn, + conn_rootdir, + fsp, + smb_fname_rel, + flags, + mode, + link_depth); + if (fd == -1) { + saved_errno = + link_errno_convert(errno); + } + } + } + + out: + + TALLOC_FREE(parent_dir); + TALLOC_FREE(smb_fname_rel); + + if (oldwd != NULL) { + int ret = vfs_ChDir(conn, oldwd); + if (ret == -1) { + smb_panic("unable to get back to old directory\n"); + } + TALLOC_FREE(oldwd); + } + if (saved_errno != 0) { + errno = saved_errno; + } + return fd; +} + /**************************************************************************** fd support routines - attempt to do a dos_open. ****************************************************************************/ -- 2.9.3 From 945e309e2be0a367d0aa5f50ff15eee45c1ed37e Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 15 Dec 2016 13:06:31 -0800 Subject: [PATCH 13/13] CVE-2017-2619: s3: smbd: Use the new non_widelink_open() function. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12496 Signed-off-by: Jeremy Allison Reviewed-by: Uri Simchoni --- source3/smbd/open.c | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/source3/smbd/open.c b/source3/smbd/open.c index 4c2ee47..08d14cb 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -638,7 +638,28 @@ NTSTATUS fd_open(struct connection_struct *conn, flags |= O_NOFOLLOW; } - fsp->fh->fd = SMB_VFS_OPEN(conn, smb_fname, fsp, flags, mode); + /* Ensure path is below share definition. */ + if (!lp_widelinks(SNUM(conn))) { + const char *conn_rootdir = SMB_VFS_CONNECTPATH(conn, + smb_fname->base_name); + if (conn_rootdir == NULL) { + return NT_STATUS_NO_MEMORY; + } + /* + * Only follow symlinks within a share + * definition. + */ + fsp->fh->fd = non_widelink_open(conn, + conn_rootdir, + fsp, + smb_fname, + flags, + mode, + 0); + } else { + fsp->fh->fd = SMB_VFS_OPEN(conn, smb_fname, fsp, flags, mode); + } + if (fsp->fh->fd == -1) { int posix_errno = link_errno_convert(errno); status = map_nt_error_from_unix(posix_errno); -- 2.9.3