티스토리 수익 글 보기

티스토리 수익 글 보기

[4.2.x] Fixed CVE-2026-25674 — Prevented potentially incorrect permi… · django/django@54b50bf · GitHub
Skip to content
/ django Public

Commit 54b50bf

Browse files
nessitanessita
committed
[4.2.x] Fixed CVE-2026-25674 — Prevented potentially incorrect permissions on file system object creation.
This fix introduces `safe_makedirs()` in the `os` utils as a safer alternative to `os.makedirs()` that avoids umask-related race conditions in multi-threaded environments. This is a workaround for python/cpython#86533 and the solution is based on the fix being proposed for CPython. Co-authored-by: Gregory P. Smith <68491+gpshead@users.noreply.github.com> Co-authored-by: Zackery Spytz <zspytz@gmail.com> Refs CVE-2020-24583 and #31921. Thanks Tarek Nakkouch for the report, and Jake Howard, Jacob Walls, and Shai Berger for reviews. Backport of 019e44f from main.
1 parent b3e8ec8 commit 54b50bf

File tree

5 files changed

+245
–17
lines changed

5 files changed

+245
–17
lines changed

‎django/core/cache/backends/filebased.py‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
from django.core.cache.backends.base import DEFAULT_TIMEOUT, BaseCache
1111
from django.core.files import locks
1212
from django.core.files.move import file_move_safe
13+
from django.utils._os import safe_makedirs
1314
from django.utils.crypto import md5
1415

1516

@@ -114,13 +115,10 @@ def _cull(self):
114115
self._delete(fname)
115116

116117
def _createdir(self):
117-
# Set the umask because os.makedirs() doesn't apply the "mode" argument
118+
# Workaround because os.makedirs() doesn't apply the "mode" argument
118119
# to intermediate-level directories.
119-
old_umask = os.umask(0o077)
120-
try:
121-
os.makedirs(self._dir, 0o700, exist_ok=True)
122-
finally:
123-
os.umask(old_umask)
120+
# https://github.com/python/cpython/issues/86533
121+
safe_makedirs(self._dir, mode=0o700, exist_ok=True)
124122

125123
def _key_to_file(self, key, version=None):
126124
"""

‎django/core/files/storage/filesystem.py‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
from django.core.files import File, locks
77
from django.core.files.move import file_move_safe
88
from django.core.signals import setting_changed
9-
from django.utils._os import safe_join
9+
from django.utils._os import safe_join, safe_makedirs
1010
from django.utils.deconstruct import deconstructible
1111
from django.utils.encoding import filepath_to_uri
1212
from django.utils.functional import cached_property
@@ -74,15 +74,10 @@ def _save(self, name, content):
7474
directory = os.path.dirname(full_path)
7575
try:
7676
if self.directory_permissions_mode is not None:
77-
# Set the umask because os.makedirs() doesn't apply the "mode"
77+
# Workaround because os.makedirs() doesn't apply the "mode"
7878
# argument to intermediate-level directories.
79-
old_umask = os.umask(0o777 & ~self.directory_permissions_mode)
80-
try:
81-
os.makedirs(
82-
directory, self.directory_permissions_mode, exist_ok=True
83-
)
84-
finally:
85-
os.umask(old_umask)
79+
# https://github.com/python/cpython/issues/86533
80+
safe_makedirs(directory, self.directory_permissions_mode, exist_ok=True)
8681
else:
8782
os.makedirs(directory, exist_ok=True)
8883
except FileExistsError:

‎django/utils/_os.py‎

Lines changed: 57 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,67 @@
11
import os
22
import tempfile
3-
from os.path import abspath, dirname, join, normcase, sep
3+
from os.path import abspath, curdir, dirname, join, normcase, sep
44
from pathlib import Path
55

66
from django.core.exceptions import SuspiciousFileOperation
77

88

9+
# Copied verbatim (minus `os.path` fixes) from:
10+
# https://github.com/python/cpython/pull/23901.
11+
# Python versions >= PY315 may include this fix, so periodic checks are needed
12+
# to remove this vendored copy of `makedirs` once solved upstream.
13+
def makedirs(name, mode=0o777, exist_ok=False, *, parent_mode=None):
14+
"""makedirs(name [, mode=0o777][, exist_ok=False][, parent_mode=None])
15+
16+
Super-mkdir; create a leaf directory and all intermediate ones. Works like
17+
mkdir, except that any intermediate path segment (not just the rightmost)
18+
will be created if it does not exist. If the target directory already
19+
exists, raise an OSError if exist_ok is False. Otherwise no exception is
20+
raised. If parent_mode is not None, it will be used as the mode for any
21+
newly-created, intermediate-level directories. Otherwise, intermediate
22+
directories are created with the default permissions (respecting umask).
23+
This is recursive.
24+
25+
"""
26+
head, tail = os.path.split(name)
27+
if not tail:
28+
head, tail = os.path.split(head)
29+
if head and tail and not os.path.exists(head):
30+
try:
31+
if parent_mode is not None:
32+
makedirs(
33+
head, mode=parent_mode, exist_ok=exist_ok, parent_mode=parent_mode
34+
)
35+
else:
36+
makedirs(head, exist_ok=exist_ok)
37+
except FileExistsError:
38+
# Defeats race condition when another thread created the path
39+
pass
40+
cdir = curdir
41+
if isinstance(tail, bytes):
42+
cdir = bytes(curdir, "ASCII")
43+
if tail == cdir: # xxx/newdir/. exists if xxx/newdir exists
44+
return
45+
try:
46+
os.mkdir(name, mode)
47+
# PY315: The call to `chmod()` is not in the CPython proposed code.
48+
# Apply `chmod()` after `mkdir()` to enforce the exact requested
49+
# permissions, since the kernel masks the mode argument with the
50+
# process umask. This guarantees consistent directory permissions
51+
# without mutating global umask state.
52+
os.chmod(name, mode)
53+
except OSError:
54+
# Cannot rely on checking for EEXIST, since the operating system
55+
# could give priority to other errors like EACCES or EROFS
56+
if not exist_ok or not os.path.isdir(name):
57+
raise
58+
59+
60+
def safe_makedirs(name, mode=0o777, exist_ok=False):
61+
"""Create directories recursively with explicit `mode` on each level."""
62+
makedirs(name=name, mode=mode, exist_ok=exist_ok, parent_mode=mode)
63+
64+
965
def safe_join(base, *paths):
1066
"""
1167
Join one or more path components to the base path component intelligently.

‎docs/releases/4.2.29.txt‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,3 +28,18 @@ the previous behavior of ``URLField.to_python()``.
2828

2929
This issue has severity "moderate" according to the :ref:`Django security
3030
policy <security-disclosure>`.
31+
32+
CVE-2026-25674: Potential incorrect permissions on newly created file system objects
33+
====================================================================================
34+
35+
Django's file-system storage and file-based cache backends used the process
36+
``umask`` to control permissions when creating directories. In multi-threaded
37+
environments, one thread's temporary umask change can affect other threads'
38+
file and directory creation, resulting in file system objects being created
39+
with unintended permissions.
40+
41+
Django now applies the requested permissions via :func:`~os.chmod` after
42+
:func:`~os.mkdir`, removing the dependency on the process-wide umask.
43+
44+
This issue has severity "low" according to the :ref:`Django security policy
45+
<security-disclosure>`.

‎tests/utils_tests/test_os_utils.py‎

Lines changed: 165 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,173 @@
11
import os
2+
import stat
3+
import sys
4+
import tempfile
25
import unittest
36
from pathlib import Path
47

58
from django.core.exceptions import SuspiciousFileOperation
6-
from django.utils._os import safe_join, to_path
9+
from django.utils._os import safe_join, safe_makedirs, to_path
10+
11+
12+
class SafeMakeDirsTests(unittest.TestCase):
13+
def setUp(self):
14+
tmp = tempfile.TemporaryDirectory()
15+
self.base = tmp.name
16+
self.addCleanup(tmp.cleanup)
17+
18+
def assertDirMode(self, path, expected):
19+
self.assertIs(os.path.isdir(path), True)
20+
if sys.platform == "win32":
21+
# Windows partially supports chmod: dirs always end up with 0o777.
22+
expected = 0o777
23+
24+
# These tests assume a typical process umask (0o022 or similar): they
25+
# create directories with modes like 0o755 and 0o700, which don't have
26+
# group/world write bits, so a typical umask doesn't change the final
27+
# permissions. On unexpected failures, check whether umask has changed.
28+
self.assertEqual(stat.S_IMODE(os.stat(path).st_mode), expected)
29+
30+
def test_creates_directory_hierarchy_with_permissions(self):
31+
path = os.path.join(self.base, "a", "b", "c")
32+
safe_makedirs(path, mode=0o755)
33+
34+
self.assertDirMode(os.path.join(self.base, "a"), 0o755)
35+
self.assertDirMode(os.path.join(self.base, "a", "b"), 0o755)
36+
self.assertDirMode(path, 0o755)
37+
38+
def test_existing_directory_exist_ok(self):
39+
path = os.path.join(self.base, "a")
40+
os.mkdir(path, 0o700)
41+
42+
safe_makedirs(path, mode=0o755, exist_ok=True)
43+
44+
self.assertDirMode(path, 0o700)
45+
46+
def test_existing_directory_exist_ok_false_raises(self):
47+
path = os.path.join(self.base, "a")
48+
os.mkdir(path)
49+
50+
with self.assertRaises(FileExistsError):
51+
safe_makedirs(path, mode=0o755, exist_ok=False)
52+
53+
def test_existing_file_at_target_raises(self):
54+
path = os.path.join(self.base, "a")
55+
with open(path, "w") as f:
56+
f.write("x")
57+
58+
with self.assertRaises(FileExistsError):
59+
safe_makedirs(path, mode=0o755, exist_ok=False)
60+
61+
with self.assertRaises(FileExistsError):
62+
safe_makedirs(path, mode=0o755, exist_ok=True)
63+
64+
def test_file_in_intermediate_path_raises(self):
65+
file_path = os.path.join(self.base, "a")
66+
with open(file_path, "w") as f:
67+
f.write("x")
68+
69+
path = os.path.join(file_path, "b")
70+
71+
expected = FileNotFoundError if sys.platform == "win32" else NotADirectoryError
72+
73+
with self.assertRaises(expected):
74+
safe_makedirs(path, mode=0o755, exist_ok=False)
75+
76+
with self.assertRaises(expected):
77+
safe_makedirs(path, mode=0o755, exist_ok=True)
78+
79+
def test_existing_parent_preserves_permissions(self):
80+
a = os.path.join(self.base, "a")
81+
b = os.path.join(a, "b")
82+
83+
os.mkdir(a, 0o700)
84+
85+
safe_makedirs(b, mode=0o755, exist_ok=False)
86+
87+
self.assertDirMode(a, 0o700)
88+
self.assertDirMode(b, 0o755)
89+
90+
c = os.path.join(a, "c")
91+
safe_makedirs(c, mode=0o750, exist_ok=True)
92+
93+
self.assertDirMode(a, 0o700)
94+
self.assertDirMode(c, 0o750)
95+
96+
def test_path_is_normalized(self):
97+
path = os.path.join(self.base, "a", "b", "..", "c")
98+
safe_makedirs(path, mode=0o755)
99+
100+
self.assertDirMode(os.path.normpath(path), 0o755)
101+
self.assertIs(os.path.isdir(os.path.join(self.base, "a", "c")), True)
102+
103+
def test_permissions_unaffected_by_process_umask(self):
104+
path = os.path.join(self.base, "a", "b", "c")
105+
# `umask()` returns the current mask, so it'll be restored on cleanup.
106+
self.addCleanup(os.umask, os.umask(0o077))
107+
108+
safe_makedirs(path, mode=0o755)
109+
110+
self.assertDirMode(os.path.join(self.base, "a"), 0o755)
111+
self.assertDirMode(os.path.join(self.base, "a", "b"), 0o755)
112+
self.assertDirMode(path, 0o755)
113+
114+
def test_permissions_correct_despite_concurrent_umask_change(self):
115+
path = os.path.join(self.base, "a", "b", "c")
116+
original_mkdir = os.mkdir
117+
# `umask()` returns the current mask, so it'll be restored on cleanup.
118+
self.addCleanup(os.umask, os.umask(0o000))
119+
120+
def mkdir_changing_umask(p, mode):
121+
# Simulate a concurrent thread changing the process umask.
122+
os.umask(0o077)
123+
original_mkdir(p, mode)
124+
125+
with unittest.mock.patch("os.mkdir", side_effect=mkdir_changing_umask):
126+
safe_makedirs(path, mode=0o755)
127+
128+
self.assertDirMode(os.path.join(self.base, "a"), 0o755)
129+
self.assertDirMode(os.path.join(self.base, "a", "b"), 0o755)
130+
self.assertDirMode(path, 0o755)
131+
132+
def test_race_condition_exist_ok_false(self):
133+
path = os.path.join(self.base, "a", "b")
134+
135+
original_mkdir = os.mkdir
136+
call_count = [0]
137+
138+
# `safe_makedirs()` calls `os.mkdir()` for each level in the path.
139+
# For path "a/b", mkdir is called twice: once for "a", once for "b".
140+
def mkdir_with_race(p, mode):
141+
call_count[0] += 1
142+
if call_count[0] == 1:
143+
original_mkdir(p, mode)
144+
else:
145+
raise FileExistsError(f"Directory exists: '{p}'")
146+
147+
with unittest.mock.patch("os.mkdir", side_effect=mkdir_with_race):
148+
with self.assertRaises(FileExistsError):
149+
safe_makedirs(path, mode=0o755, exist_ok=False)
150+
151+
def test_race_condition_exist_ok_true(self):
152+
path = os.path.join(self.base, "a", "b")
153+
154+
original_mkdir = os.mkdir
155+
call_count = [0]
156+
157+
def mkdir_with_race(p, mode):
158+
call_count[0] += 1
159+
if call_count[0] == 1:
160+
original_mkdir(p, mode)
161+
else:
162+
# Simulate other thread creating the directory during the race.
163+
# The directory needs to exist for `exist_ok=True` to succeed.
164+
original_mkdir(p, mode)
165+
raise FileExistsError(f"Directory exists: '{p}'")
166+
167+
with unittest.mock.patch("os.mkdir", side_effect=mkdir_with_race):
168+
safe_makedirs(path, mode=0o755, exist_ok=True)
169+
170+
self.assertIs(os.path.isdir(path), True)
7171

8172

9173
class SafeJoinTests(unittest.TestCase):

0 commit comments

Comments
 (0)