diff options
| author | Jeremy Allison <jra@samba.org> | 2021-12-07 14:39:42 -0800 |
|---|---|---|
| committer | Jule Anger <janger@samba.org> | 2022-01-31 12:23:53 +0100 |
| commit | 9371ace08e603c745be14d6131b7a7713b36e782 (patch) | |
| tree | 5c090eec1f3a09e6e1f0e3b52bfbc4cb6ddff637 | |
| parent | 66774e97e200d686be9c54739dc67ff0ed56af6f (diff) | |
| download | samba-9371ace08e603c745be14d6131b7a7713b36e782.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 0d82085870c..56ebdd9f370 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -30,6 +30,9 @@ #include "smbd/smbd.h" #include "smbd/globals.h" +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; @@ -529,6 +532,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 { /* |