From: Volker Lendecke <vl@samba.org>
Date: Tue, 5 Sep 2023 13:25:07 +0000 (+0200)
Subject: smbd: Make open_file() a bit safer
X-Git-Tag: tevent-0.16.0~224
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=6ec031b2d146962a414da69694d298c00df0c517;p=thirdparty%2Fsamba.git

smbd: Make open_file() a bit safer

Move adding O_RDWR before the check for read only shares. I haven't
been able to pass this condition through SMB, but in any case we
should not accidentially open with O_RDWR in the !CAN_WRITE(conn)
case.

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>
---

diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index 973e656d128..6e18844bb16 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -1335,6 +1335,24 @@ static NTSTATUS open_file(struct smb_request *req,
 		return NT_STATUS_FILE_IS_A_DIRECTORY;
 	}
 
+	/*
+	 * This little piece of insanity is inspired by the
+	 * fact that an NT client can open a file for O_RDONLY,
+	 * but set the create disposition to FILE_EXISTS_TRUNCATE.
+	 * If the client *can* write to the file, then it expects to
+	 * truncate the file, even though it is opening for readonly.
+	 * Quicken uses this stupid trick in backup file creation...
+	 * Thanks *greatly* to "David W. Chapman Jr." <dwcjr@inethouston.net>
+	 * for helping track this one down. It didn't bite us in 2.0.x
+	 * as we always opened files read-write in that release. JRA.
+	 */
+
+	if (((flags & O_ACCMODE) == O_RDONLY) && (flags & O_TRUNC)) {
+		DBG_DEBUG("truncate requested on read-only open for file %s\n",
+			  smb_fname_str_dbg(smb_fname));
+		local_flags = (flags & ~O_ACCMODE) | O_RDWR;
+	}
+
 	/* Check permissions */
 
 	/*
@@ -1363,24 +1381,6 @@ static NTSTATUS open_file(struct smb_request *req,
 		local_flags &= ~(O_CREAT | O_EXCL);
 	}
 
-	/*
-	 * This little piece of insanity is inspired by the
-	 * fact that an NT client can open a file for O_RDONLY,
-	 * but set the create disposition to FILE_EXISTS_TRUNCATE.
-	 * If the client *can* write to the file, then it expects to
-	 * truncate the file, even though it is opening for readonly.
-	 * Quicken uses this stupid trick in backup file creation...
-	 * Thanks *greatly* to "David W. Chapman Jr." <dwcjr@inethouston.net>
-	 * for helping track this one down. It didn't bite us in 2.0.x
-	 * as we always opened files read-write in that release. JRA.
-	 */
-
-	if (((flags & O_ACCMODE) == O_RDONLY) && (flags & O_TRUNC)) {
-		DEBUG(10,("open_file: truncate requested on read-only open "
-			  "for file %s\n", smb_fname_str_dbg(smb_fname)));
-		local_flags = (flags & ~O_ACCMODE)|O_RDWR;
-	}
-
 	if ((open_access_mask & need_fd_mask) || creating ||
 	    (flags & O_TRUNC)) {
 		open_fd = true;