From: Jeremy Allison <jra@samba.org>
Date: Mon, 25 Oct 2021 19:21:37 +0000 (-0700)
Subject: s3: smbd: Fix rmdir_internals() to do an early return if lp_delete_veto_files() is... 
X-Git-Tag: ldb-2.5.0~336
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=a37d16e7c55f85e3f2c9c8614755ea6307092d5f;p=thirdparty%2Fsamba.git

s3: smbd: Fix rmdir_internals() to do an early return if lp_delete_veto_files() is not set.

Fix the comments to match what the code actually does. The
exit at the end of the scan directory loop if we find a client
visible filename is a change in behavior, but the previous
behavior (not exist on visible filename, but delete it) was
a bug and in non-tested code. Now it's testd.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14879

Signed-off-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>
---

diff --git a/source3/smbd/close.c b/source3/smbd/close.c
index 45beaad84ff..882a7bbbeb3 100644
--- a/source3/smbd/close.c
+++ b/source3/smbd/close.c
@@ -962,8 +962,6 @@ static NTSTATUS rmdir_internals(TALLOC_CTX *ctx, struct files_struct *fsp)
 	struct smb_filename *smb_dname = fsp->fsp_name;
 	struct smb_filename *parent_fname = NULL;
 	struct smb_filename *at_fname = NULL;
-	const struct loadparm_substitution *lp_sub =
-		loadparm_s3_global_substitution();
 	SMB_STRUCT_STAT st;
 	const char *dname = NULL;
 	char *talloced = NULL;
@@ -1023,9 +1021,7 @@ static NTSTATUS rmdir_internals(TALLOC_CTX *ctx, struct files_struct *fsp)
 		return NT_STATUS_OK;
 	}
 
-	if (!((errno == ENOTEMPTY) || (errno == EEXIST)) ||
-	    !*lp_veto_files(talloc_tos(), lp_sub, SNUM(conn)))
-	{
+	if (!((errno == ENOTEMPTY) || (errno == EEXIST))) {
 		DEBUG(3,("rmdir_internals: couldn't remove directory %s : "
 			 "%s\n", smb_fname_str_dbg(smb_dname),
 			 strerror(errno)));
@@ -1033,11 +1029,21 @@ static NTSTATUS rmdir_internals(TALLOC_CTX *ctx, struct files_struct *fsp)
 		return map_nt_error_from_unix(errno);
 	}
 
+	/*
+	 * Here we know the initial directory unlink failed with
+	 * ENOTEMPTY or EEXIST so we know there are objects within.
+	 * If we don't have permission to delete files non
+	 * visible to the client just fail the directory delete.
+	 */
+
+	if (!lp_delete_veto_files(SNUM(conn))) {
+		errno = ENOTEMPTY;
+		goto err;
+	}
+
 	/*
 	 * Check to see if the only thing in this directory are
-	 * vetoed files/directories. If so then delete them and
-	 * retry. If we fail to delete any of them (and we *don't*
-	 * do a recursive delete) then fail the rmdir.
+	 * files non-visible to the client. If not, fail the delete.
 	 */
 
 	dir_hnd = OpenDir(talloc_tos(), conn, smb_dname, NULL, 0);
@@ -1130,16 +1136,18 @@ static NTSTATUS rmdir_internals(TALLOC_CTX *ctx, struct files_struct *fsp)
 			continue;
 		}
 
+		/*
+		 * We found a client visible name.
+		 * We cannot delete this directory.
+		 */
+		DBG_DEBUG("got name %s - "
+			"can't delete directory %s\n",
+			dname,
+			fsp_str_dbg(fsp));
 		TALLOC_FREE(talloced);
 		TALLOC_FREE(fullname);
 		TALLOC_FREE(smb_dname_full);
 		TALLOC_FREE(direntry_fname);
-	}
-
-	/* We only have veto files/directories.
-	 * Are we allowed to delete them ? */
-
-	if (!lp_delete_veto_files(SNUM(conn))) {
 		errno = ENOTEMPTY;
 		goto err;
 	}