id	summary	reporter	owner	description	type	status	component	version	severity	resolution	keywords	cc	stage	has_patch	needs_docs	needs_tests	needs_better_patch	easy	ui_ux
37444	Prevent overly broad destination permissions during file_move_safe() fallback copying	Sarah Boyce	Sarah Boyce	"When `django.core.files.move.file_move_safe()` cannot use `os.rename()`, for example when moving across filesystems, it falls back to copying the file contents. See https://github.com/django/django/blob/3d3d943b7442f1f5278ed876331a06047fa96ea8/django/core/files/move.py#L52

The fallback creates the destination using `os.open()` with `O_CREAT` but without an explicit mode. This defaults to `0o777`, subject to the process umask. The source permissions are applied using `copystat()` only after copying completes.

As a consequence, a source file with permissions of `0o600` (owner read/write only) can have a destination with permissions of `0o755` (owner read/write/execute; group and others read/execute) during copying when the process umask is `0o022` (removing write permission for group and others).

The destination should be created with permissions that do not expose the source contents more broadly during copying.

See test for behavior demonstration:
{{{#!diff
--- a/tests/files/tests.py
+++ b/tests/files/tests.py
@@ -1,6 +1,7 @@
 import errno
 import gzip
 import os
+import stat
 import struct
 import tempfile
 import unittest
@@ -520,6 +521,49 @@ class FileMoveSafeTests(unittest.TestCase):
 
         self.assertEqual(content, b""content"")
 
+    @unittest.skipUnless(os.name == ""posix"", ""Requires POSIX permissions."")
+    def test_file_move_preserves_private_permissions_during_copy(self):
+        real_write = os.write
+
+        def recording_write(fd, content):
+            written = real_write(fd, content)
+            if written:
+                observed_modes.append(stat.S_IMODE(os.fstat(fd).st_mode))
+            return written
+
+        # umask() returns the current mask, so it'll be restored on cleanup.
+        self.addCleanup(os.umask, os.umask(0o000))
+        destination_creation_mode = 0o600
+
+        for source_mode in (0o600, 0o640):
+            with (
+                self.subTest(source_mode=source_mode),
+                tempfile.TemporaryDirectory() as temp_dir,
+            ):
+                observed_modes = []
+                source = Path(temp_dir) / ""source""
+                destination = Path(temp_dir) / ""destination""
+                source.write_bytes(b""private content"")
+                source.chmod(source_mode)
+
+                with (
+                    mock.patch(
+                        ""django.core.files.move.os.rename"",
+                        side_effect=OSError(errno.EXDEV),
+                    ),
+                    mock.patch(
+                        ""django.core.files.move.os.write"",
+                        side_effect=recording_write,
+                    ),
+                ):
+                    file_move_safe(source, destination)
+
+                # Keep contents private until copying has finished.
+                self.assertEqual(set(observed_modes), {destination_creation_mode})
+                self.assertEqual(stat.S_IMODE(destination.stat().st_mode), source_mode)
+                self.assertEqual(destination.read_bytes(), b""private content"")
+                self.assertFalse(source.exists())
+
 
 class SpooledTempTests(unittest.TestCase):
}}}

This was reported privately to the Django security team by ""Squirt1e, Atum, N0b0dy, Gh0u1L5 Please credit Squirt1e, Atum, N0b0dy, and Gh0u1L5"", which assessed it as a bug suitable for public hardening."	Bug	assigned	File uploads/storage	6.1	Normal		not-security		Unreviewed	1	0	0	0	0	0