diff options
author | Jeremy Allison <jra@samba.org> | 2021-12-07 14:39:42 -0800 |
---|---|---|
committer | Stefan Metzmacher <metze@samba.org> | 2022-01-31 14:26:10 +0000 |
commit | 86157b3c7bfda64060ed4fbe23711aaa571be092 (patch) | |
tree | 7a9e3a232a2b725ebc5c5b228229127ac123c942 | |
parent | f4202a0bccd5d52aa5448748cdd2d67a68738fc0 (diff) | |
download | samba-86157b3c7bfda64060ed4fbe23711aaa571be092.tar.gz |
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 <jra@samba.org>
-rw-r--r-- | selftest/knownfail.d/symlink_traversal | 2 | ||||
-rw-r--r-- | source3/smbd/filename.c | 36 |
2 files changed, 36 insertions, 2 deletions
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 53c58d6e80a..ef382b43bd6 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -36,6 +36,9 @@ static int get_real_filename(connection_struct *conn, TALLOC_CTX *mem_ctx, char **found_name); +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; @@ -542,6 +545,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 { /* |