Ensure vfs_chown_fsp() is safe against races.

3 files changed, 74 insertions, 6 deletions

diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index b7c8540da1..14e6bf9b4e 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -2563,7 +2563,7 @@ static NTSTATUS mkdir_internal(connection_struct *conn,
  Ensure we didn't get symlink raced on opening a directory.
 ****************************************************************************/
 
-static bool check_same_stat(const SMB_STRUCT_STAT *sbuf1,
+bool check_same_stat(const SMB_STRUCT_STAT *sbuf1,
 			const SMB_STRUCT_STAT *sbuf2)
 {
 	if (sbuf1->st_ex_uid != sbuf2->st_ex_uid ||
diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h
index c097202676..36eea5e16e 100644
--- a/source3/smbd/proto.h
+++ b/source3/smbd/proto.h
@@ -567,6 +567,8 @@ void remove_deferred_open_entry(struct file_id id, uint64_t mid,
 NTSTATUS open_file_fchmod(connection_struct *conn,
 			  struct smb_filename *smb_fname,
 			  files_struct **result);
+bool check_same_stat(const SMB_STRUCT_STAT *sbuf1,
+			const SMB_STRUCT_STAT *sbuf2);
 NTSTATUS create_directory(connection_struct *conn, struct smb_request *req,
 			  struct smb_filename *smb_dname);
 void msg_file_was_renamed(struct messaging_context *msg,
diff --git a/source3/smbd/vfs.c b/source3/smbd/vfs.c
index d8c57a6a86..0438f7934e 100644
--- a/source3/smbd/vfs.c
+++ b/source3/smbd/vfs.c
@@ -1455,6 +1455,11 @@ int smb_vfs_call_lchown(struct vfs_handle_struct *handle, const char *path,
 NTSTATUS vfs_chown_fsp(files_struct *fsp, uid_t uid, gid_t gid)
 {
 	int ret;
+	bool as_root = false;
+	const char *path;
+	char *saved_dir = NULL;
+	char *parent_dir = NULL;
+	NTSTATUS status;
 
 	if (fsp->fh->fd != -1) {
 		/* Try fchown. */
@@ -1467,19 +1472,80 @@ NTSTATUS vfs_chown_fsp(files_struct *fsp, uid_t uid, gid_t gid)
 		}
 	}
 
-	if (fsp->posix_open) {
+	as_root = (geteuid() == 0);
+
+	if (as_root) {
+		/*
+		 * We are being asked to chown as root. Make
+		 * sure we chdir() into the path to pin it,
+		 * and always act using lchown to ensure we
+		 * don't deref any symbolic links.
+		 */
+		const char *final_component = NULL;
+		struct smb_filename local_fname;
+
+		saved_dir = vfs_GetWd(talloc_tos(),fsp->conn);
+		if (!saved_dir) {
+			status = map_nt_error_from_unix(errno);
+			DEBUG(0,("vfs_chown_fsp: failed to get "
+				"current working directory. Error was %s\n",
+				strerror(errno)));
+			return status;
+		}
+
+		if (!parent_dirname(talloc_tos(),
+				fsp->fsp_name->base_name,
+				&parent_dir,
+				&final_component)) {
+			return NT_STATUS_NO_MEMORY;
+		}
+
+		/* cd into the parent dir to pin it. */
+		ret = SMB_VFS_CHDIR(fsp->conn, parent_dir);
+		if (ret == -1) {
+			return map_nt_error_from_unix(errno);
+		}
+
+		ZERO_STRUCT(local_fname);
+		local_fname.base_name = CONST_DISCARD(char *,final_component);
+
+		/* Must use lstat here. */
+		ret = SMB_VFS_LSTAT(fsp->conn, &local_fname);
+		if (ret == -1) {
+			return map_nt_error_from_unix(errno);
+		}
+
+		/* Ensure it matches the fsp stat. */
+		if (!check_same_stat(&local_fname.st, &fsp->fsp_name->st)) {
+                        return NT_STATUS_ACCESS_DENIED;
+                }
+                path = final_component;
+        } else {
+                path = fsp->fsp_name->base_name;
+        }
+
+	if (fsp->posix_open || as_root) {
 		ret = SMB_VFS_LCHOWN(fsp->conn,
-			fsp->fsp_name->base_name,
+			path,
 			uid, gid);
 	} else {
 		ret = SMB_VFS_CHOWN(fsp->conn,
-			fsp->fsp_name->base_name,
+			path,
 			uid, gid);
 	}
+
 	if (ret == 0) {
-		return NT_STATUS_OK;
+		status = NT_STATUS_OK;
+	} else {
+		status = map_nt_error_from_unix(errno);
+	}
+
+	if (as_root) {
+		vfs_ChDir(fsp->conn,saved_dir);
+		TALLOC_FREE(saved_dir);
+		TALLOC_FREE(parent_dir);
 	}
-	return map_nt_error_from_unix(errno);
+	return status;
 }
 
 int smb_vfs_call_chdir(struct vfs_handle_struct *handle, const char *path)