]> git.ipfire.org Git - thirdparty/samba.git/commitdiff
smbd: Make open_file() a bit safer
authorVolker Lendecke <vl@samba.org>
Tue, 5 Sep 2023 13:25:07 +0000 (15:25 +0200)
committerRalph Boehme <slow@samba.org>
Thu, 5 Oct 2023 12:58:33 +0000 (12:58 +0000)
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>
source3/smbd/open.c

index 973e656d1285815084098a6372b5b4516eeddc29..6e18844bb16ac26bd63126440e6a262b8e6ad472 100644 (file)
@@ -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;