Privilege escalation / Incorrect permissions due to race condition when creating filesystem objects

# Proof of concept (PoC) illustrating a potential race around process umask affecting directory permissions during multi-threaded creation. # This demonstrates the concept rather than a guaranteed exploit since exact timing can vary by OS/CPU. import os import threading import time import tempfile import stat base = tempfile.mkdtemp(prefix="poc_umask_") def create_dirs_with_mode(path, mode): # This mimics the problematic pattern: creating dirs with a given mode # without atomically enforcing permissions across all levels. os.makedirs(path, mode=mode, exist_ok=True) def flip_umask(): # Rapidly flip umask to induce race conditions during directory creation for _ in range(20): os.umask(0o077) time.sleep(0.005) os.umask(0o000) time.sleep(0.005) def run_poc(): # Target path with two levels to illustrate intermediate directories target = os.path.join(base, "a", "b") t1 = threading.Thread(target=flip_umask) t2 = threading.Thread(target=create_dirs_with_mode, args=(target, 0o700)) t1.start() t2.start() t1.join() t2.join() # Show final permissions observed on the created directories for d, sub in [(base, ""), (os.path.join(base, "a"), "a"), (os.path.join(base, "a", "b"), "a/b")]: if os.path.isdir(os.path.join(d, sub)): p = os.path.join(d, sub) mode = stat.S_IMODE(os.stat(p).st_mode) print(p, oct(mode)) if __name__ == "__main__": run_poc() # Expected outcome (poor-man's illustration): # Depending on timing, intermediate directory permissions may reflect # the effective umask at the moment each mkdir occurred, potentially yielding # permissions that differ from the intended explicit mode (0o700). # With the fix in place (safe_makedirs + explicit chmod), such race-induced # discrepancies would be eliminated.

diff --git a/django/core/cache/backends/filebased.py b/django/core/cache/backends/filebased.py index 862a8b57d93b..9f2ad48ac886 100644 --- a/django/core/cache/backends/filebased.py +++ b/django/core/cache/backends/filebased.py @@ -12,6 +12,7 @@ from django.core.cache.backends.base import DEFAULT_TIMEOUT, BaseCache from django.core.files import locks from django.core.files.move import file_move_safe +from django.utils._os import safe_makedirs class FileBasedCache(BaseCache): @@ -115,13 +116,10 @@ def _cull(self): self._delete(fname) def _createdir(self): - # Set the umask because os.makedirs() doesn't apply the "mode" argument + # Workaround because os.makedirs() doesn't apply the "mode" argument # to intermediate-level directories. - old_umask = os.umask(0o077) - try: - os.makedirs(self._dir, 0o700, exist_ok=True) - finally: - os.umask(old_umask) + # https://github.com/python/cpython/issues/86533 + safe_makedirs(self._dir, mode=0o700, exist_ok=True) def _key_to_file(self, key, version=None): """ diff --git a/django/core/files/storage/filesystem.py b/django/core/files/storage/filesystem.py index 9592bff00884..867f2e477c10 100644 --- a/django/core/files/storage/filesystem.py +++ b/django/core/files/storage/filesystem.py @@ -6,7 +6,7 @@ from django.core.files import File, locks from django.core.files.move import file_move_safe from django.core.signals import setting_changed -from django.utils._os import safe_join +from django.utils._os import safe_join, safe_makedirs from django.utils.deconstruct import deconstructible from django.utils.encoding import filepath_to_uri from django.utils.functional import cached_property @@ -72,15 +72,10 @@ def _save(self, name, content): directory = os.path.dirname(full_path) try: if self.directory_permissions_mode is not None: - # Set the umask because os.makedirs() doesn't apply the "mode" + # Workaround because os.makedirs() doesn't apply the "mode" # argument to intermediate-level directories. - old_umask = os.umask(0o777 & ~self.directory_permissions_mode) - try: - os.makedirs( - directory, self.directory_permissions_mode, exist_ok=True - ) - finally: - os.umask(old_umask) + # https://github.com/python/cpython/issues/86533 + safe_makedirs(directory, self.directory_permissions_mode, exist_ok=True) else: os.makedirs(directory, exist_ok=True) except FileExistsError: diff --git a/django/utils/_os.py b/django/utils/_os.py index 5cd8c566a8d7..f2969c5e0067 100644 --- a/django/utils/_os.py +++ b/django/utils/_os.py @@ -1,11 +1,67 @@ import os import tempfile -from os.path import abspath, dirname, join, normcase, sep +from os.path import abspath, curdir, dirname, join, normcase, sep from pathlib import Path from django.core.exceptions import SuspiciousFileOperation +# Copied verbatim (minus `os.path` fixes) from: +# https://github.com/python/cpython/pull/23901. +# Python versions >= PY315 may include this fix, so periodic checks are needed +# to remove this vendored copy of `makedirs` once solved upstream. +def makedirs(name, mode=0o777, exist_ok=False, *, parent_mode=None): + """makedirs(name [, mode=0o777][, exist_ok=False][, parent_mode=None]) + + Super-mkdir; create a leaf directory and all intermediate ones. Works like + mkdir, except that any intermediate path segment (not just the rightmost) + will be created if it does not exist. If the target directory already + exists, raise an OSError if exist_ok is False. Otherwise no exception is + raised. If parent_mode is not None, it will be used as the mode for any + newly-created, intermediate-level directories. Otherwise, intermediate + directories are created with the default permissions (respecting umask). + This is recursive. + + """ + head, tail = os.path.split(name) + if not tail: + head, tail = os.path.split(head) + if head and tail and not os.path.exists(head): + try: + if parent_mode is not None: + makedirs( + head, mode=parent_mode, exist_ok=exist_ok, parent_mode=parent_mode + ) + else: + makedirs(head, exist_ok=exist_ok) + except FileExistsError: + # Defeats race condition when another thread created the path + pass + cdir = curdir + if isinstance(tail, bytes): + cdir = bytes(curdir, "ASCII") + if tail == cdir: # xxx/newdir/. exists if xxx/newdir exists + return + try: + os.mkdir(name, mode) + # PY315: The call to `chmod()` is not in the CPython proposed code. + # Apply `chmod()` after `mkdir()` to enforce the exact requested + # permissions, since the kernel masks the mode argument with the + # process umask. This guarantees consistent directory permissions + # without mutating global umask state. + os.chmod(name, mode) + except OSError: + # Cannot rely on checking for EEXIST, since the operating system + # could give priority to other errors like EACCES or EROFS + if not exist_ok or not os.path.isdir(name): + raise + + +def safe_makedirs(name, mode=0o777, exist_ok=False): + """Create directories recursively with explicit `mode` on each level.""" + makedirs(name=name, mode=mode, exist_ok=exist_ok, parent_mode=mode) + + def safe_join(base, *paths): """ Join one or more path components to the base path component intelligently. diff --git a/docs/releases/4.2.29.txt b/docs/releases/4.2.29.txt index b780264929de..71170a576398 100644 --- a/docs/releases/4.2.29.txt +++ b/docs/releases/4.2.29.txt @@ -28,3 +28,18 @@ the previous behavior of ``URLField.to_python()``. This issue has severity "moderate" according to the :ref:`Django security policy <security-disclosure>`. + +CVE-2026-25674: Potential incorrect permissions on newly created file system objects +==================================================================================== + +Django's file-system storage and file-based cache backends used the process +``umask`` to control permissions when creating directories. In multi-threaded +environments, one thread's temporary umask change can affect other threads' +file and directory creation, resulting in file system objects being created +with unintended permissions. + +Django now applies the requested permissions via :func:`~os.chmod` after +:func:`~os.mkdir`, removing the dependency on the process-wide umask. + +This issue has severity "low" according to the :ref:`Django security policy +<security-disclosure>`. diff --git a/docs/releases/5.2.12.txt b/docs/releases/5.2.12.txt index be2c7bc80719..177bfd1cedec 100644 --- a/docs/releases/5.2.12.txt +++ b/docs/releases/5.2.12.txt @@ -30,6 +30,21 @@ the previous behavior of ``URLField.to_python()``. This issue has severity "moderate" according to the :ref:`Django security policy <security-disclosure>`. +CVE-2026-25674: Potential incorrect permissions on newly created file system objects +==================================================================================== + +Django's file-system storage and file-based cache backends used the process +``umask`` to control permissions when creating directories. In multi-threaded +environments, one thread's temporary umask change can affect other threads' +file and directory creation, resulting in file system objects being created +with unintended permissions. + +Django now applies the requested permissions via :func:`~os.chmod` after +:func:`~os.mkdir`, removing the dependency on the process-wide umask. + +This issue has severity "low" according to the :ref:`Django security policy +<security-disclosure>`. + Bugfixes ======== diff --git a/docs/releases/6.0.3.txt b/docs/releases/6.0.3.txt index 6750385c1eea..31f12b179232 100644 --- a/docs/releases/6.0.3.txt +++ b/docs/releases/6.0.3.txt @@ -29,6 +29,21 @@ the previous behavior of ``URLField.to_python()``. This issue has severity "moderate" according to the :ref:`Django security policy <security-disclosure>`. +CVE-2026-25674: Potential incorrect permissions on newly created file system objects +==================================================================================== + +Django's file-system storage and file-based cache backends used the process +``umask`` to control permissions when creating directories. In multi-threaded +environments, one thread's temporary umask change can affect other threads' +file and directory creation, resulting in file system objects being created +with unintended permissions. + +Django now applies the requested permissions via :func:`~os.chmod` after +:func:`~os.mkdir`, removing the dependency on the process-wide umask. + +This issue has severity "low" according to the :ref:`Django security policy +<security-disclosure>`. + Bugfixes ======== diff --git a/tests/utils_tests/test_os_utils.py b/tests/utils_tests/test_os_utils.py index 7204167688dd..290e418e64cd 100644 --- a/tests/utils_tests/test_os_utils.py +++ b/tests/utils_tests/test_os_utils.py @@ -1,9 +1,173 @@ import os +import stat +import sys +import tempfile import unittest from pathlib import Path from django.core.exceptions import SuspiciousFileOperation -from django.utils._os import safe_join, to_path +from django.utils._os import safe_join, safe_makedirs, to_path + + +class SafeMakeDirsTests(unittest.TestCase): + def setUp(self): + tmp = tempfile.TemporaryDirectory() + self.base = tmp.name + self.addCleanup(tmp.cleanup) + + def assertDirMode(self, path, expected): + self.assertIs(os.path.isdir(path), True) + if sys.platform == "win32": + # Windows partially supports chmod: dirs always end up with 0o777. + expected = 0o777 + + # These tests assume a typical process umask (0o022 or similar): they + # create directories with modes like 0o755 and 0o700, which don't have + # group/world write bits, so a typical umask doesn't change the final + # permissions. On unexpected failures, check whether umask has changed. + self.assertEqual(stat.S_IMODE(os.stat(path).st_mode), expected) + + def test_creates_directory_hierarchy_with_permissions(self): + path = os.path.join(self.base, "a", "b", "c") + safe_makedirs(path, mode=0o755) + + self.assertDirMode(os.path.join(self.base, "a"), 0o755) + self.assertDirMode(os.path.join(self.base, "a", "b"), 0o755) + self.assertDirMode(path, 0o755) + + def test_existing_directory_exist_ok(self): + path = os.path.join(self.base, "a") + os.mkdir(path, 0o700) + + safe_makedirs(path, mode=0o755, exist_ok=True) + + self.assertDirMode(path, 0o700) + + def test_existing_directory_exist_ok_false_raises(self): + path = os.path.join(self.base, "a") + os.mkdir(path) + + with self.assertRaises(FileExistsError): + safe_makedirs(path, mode=0o755, exist_ok=False) + + def test_existing_file_at_target_raises(self): + path = os.path.join(self.base, "a") + with open(path, "w") as f: + f.write("x") + + with self.assertRaises(FileExistsError): + safe_makedirs(path, mode=0o755, exist_ok=False) + + with self.assertRaises(FileExistsError): + safe_makedirs(path, mode=0o755, exist_ok=True) + + def test_file_in_intermediate_path_raises(self): + file_path = os.path.join(self.base, "a") + with open(file_path, "w") as f: + f.write("x") + + path = os.path.join(file_path, "b") + + expected = FileNotFoundError if sys.platform == "win32" else NotADirectoryError + + with self.assertRaises(expected): + safe_makedirs(path, mode=0o755, exist_ok=False) + + with self.assertRaises(expected): + safe_makedirs(path, mode=0o755, exist_ok=True) + + def test_existing_parent_preserves_permissions(self): + a = os.path.join(self.base, "a") + b = os.path.join(a, "b") + + os.mkdir(a, 0o700) + + safe_makedirs(b, mode=0o755, exist_ok=False) + + self.assertDirMode(a, 0o700) + self.assertDirMode(b, 0o755) + + c = os.path.join(a, "c") + safe_makedirs(c, mode=0o750, exist_ok=True) + + self.assertDirMode(a, 0o700) + self.assertDirMode(c, 0o750) + + def test_path_is_normalized(self): + path = os.path.join(self.base, "a", "b", "..", "c") + safe_makedirs(path, mode=0o755) + + self.assertDirMode(os.path.normpath(path), 0o755) + self.assertIs(os.path.isdir(os.path.join(self.base, "a", "c")), True) + + def test_permissions_unaffected_by_process_umask(self): + path = os.path.join(self.base, "a", "b", "c") + # `umask()` returns the current mask, so it'll be restored on cleanup. + self.addCleanup(os.umask, os.umask(0o077)) + + safe_makedirs(path, mode=0o755) + + self.assertDirMode(os.path.join(self.base, "a"), 0o755) + self.assertDirMode(os.path.join(self.base, "a", "b"), 0o755) + self.assertDirMode(path, 0o755) + + def test_permissions_correct_despite_concurrent_umask_change(self): + path = os.path.join(self.base, "a", "b", "c") + original_mkdir = os.mkdir + # `umask()` returns the current mask, so it'll be restored on cleanup. + self.addCleanup(os.umask, os.umask(0o000)) + + def mkdir_changing_umask(p, mode): + # Simulate a concurrent thread changing the process umask. + os.umask(0o077) + original_mkdir(p, mode) + + with unittest.mock.patch("os.mkdir", side_effect=mkdir_changing_umask): + safe_makedirs(path, mode=0o755) + + self.assertDirMode(os.path.join(self.base, "a"), 0o755) + self.assertDirMode(os.path.join(self.base, "a", "b"), 0o755) + self.assertDirMode(path, 0o755) + + def test_race_condition_exist_ok_false(self): + path = os.path.join(self.base, "a", "b") + + original_mkdir = os.mkdir + call_count = [0] + + # `safe_makedirs()` calls `os.mkdir()` for each level in the path. + # For path "a/b", mkdir is called twice: once for "a", once for "b". + def mkdir_with_race(p, mode): + call_count[0] += 1 + if call_count[0] == 1: + original_mkdir(p, mode) + else: + raise FileExistsError(f"Directory exists: '{p}'") + + with unittest.mock.patch("os.mkdir", side_effect=mkdir_with_race): + with self.assertRaises(FileExistsError): + safe_makedirs(path ... [truncated]