Prevent overly broad destination permissions during file_move_safe() fallback copying

[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:

tests/files/tests.py

@@ -1 +1 @@
 import errno
 import gzip
 import os
+import stat
 import struct
 import tempfile
 import unittest
@@ -520 +521 @@
 
         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.