Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 91e3669

Browse filesBrowse files
zoobaambv
zooba
and
ambv
authored
[3.8] gh-118486: Support mkdir(mode=0o700) on Windows (GH-118488) (GH-118742)
Co-authored-by: Łukasz Langa <lukasz@langa.pl>
1 parent 29c9728 commit 91e3669
Copy full SHA for 91e3669

File tree

6 files changed

+108
-3
lines changed
Filter options

6 files changed

+108
-3
lines changed

‎Doc/library/os.rst

Copy file name to clipboardExpand all lines: Doc/library/os.rst
+7Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1909,6 +1909,10 @@ features:
19091909
platform-dependent. On some platforms, they are ignored and you should call
19101910
:func:`chmod` explicitly to set them.
19111911

1912+
On Windows, a *mode* of ``0o700`` is specifically handled to apply access
1913+
control to the new directory such that only the current user and
1914+
administrators have access. Other values of *mode* are ignored.
1915+
19121916
This function can also support :ref:`paths relative to directory descriptors
19131917
<dir_fd>`.
19141918

@@ -1923,6 +1927,9 @@ features:
19231927
.. versionchanged:: 3.6
19241928
Accepts a :term:`path-like object`.
19251929

1930+
.. versionchanged:: 3.8.20
1931+
Windows now handles a *mode* of ``0o700``.
1932+
19261933

19271934
.. function:: makedirs(name, mode=0o777, exist_ok=False)
19281935

‎Doc/whatsnew/3.8.rst

Copy file name to clipboardExpand all lines: Doc/whatsnew/3.8.rst
+16Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1046,6 +1046,13 @@ treat junctions as links.
10461046

10471047
(Contributed by Steve Dower in :issue:`37834`.)
10481048

1049+
As of 3.8.20, :func:`os.mkdir` and :func:`os.makedirs` on Windows now support
1050+
passing a *mode* value of ``0o700`` to apply access control to the new
1051+
directory. This implicitly affects :func:`tempfile.mkdtemp` and is a
1052+
mitigation for CVE-2024-4030. Other values for *mode* continue to be
1053+
ignored.
1054+
(Contributed by Steve Dower in :gh:`118486`.)
1055+
10491056

10501057
os.path
10511058
-------
@@ -1252,6 +1259,15 @@ in a standardized and extensible format, and offers several other benefits.
12521259
(Contributed by C.A.M. Gerlach in :issue:`36268`.)
12531260

12541261

1262+
tempfile
1263+
--------
1264+
1265+
As of 3.8.20 on Windows, the default mode ``0o700`` used by
1266+
:func:`tempfile.mkdtemp` now limits access to the new directory due to
1267+
changes to :func:`os.mkdir`. This is a mitigation for CVE-2024-4030.
1268+
(Contributed by Steve Dower in :gh:`118486`.)
1269+
1270+
12551271
threading
12561272
---------
12571273

‎Lib/test/test_os.py

Copy file name to clipboardExpand all lines: Lib/test/test_os.py
+12Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1380,6 +1380,18 @@ def test_exist_ok_existing_regular_file(self):
13801380
self.assertRaises(OSError, os.makedirs, path, exist_ok=True)
13811381
os.remove(path)
13821382

1383+
@unittest.skipUnless(os.name == 'nt', "requires Windows")
1384+
def test_win32_mkdir_700(self):
1385+
base = support.TESTFN
1386+
path = os.path.abspath(os.path.join(support.TESTFN, 'dir'))
1387+
os.mkdir(path, mode=0o700)
1388+
out = subprocess.check_output(["cacls.exe", path, "/s"], encoding="oem")
1389+
os.rmdir(path)
1390+
self.assertEqual(
1391+
out.strip(),
1392+
f'{path} "D:P(A;OICI;FA;;;SY)(A;OICI;FA;;;BA)(A;OICI;FA;;;OW)"',
1393+
)
1394+
13831395
def tearDown(self):
13841396
path = os.path.join(support.TESTFN, 'dir1', 'dir2', 'dir3',
13851397
'dir4', 'dir5', 'dir6')

‎Lib/test/test_tempfile.py

Copy file name to clipboardExpand all lines: Lib/test/test_tempfile.py
+28Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import contextlib
1212
import stat
1313
import weakref
14+
import subprocess
1415
from unittest import mock
1516

1617
import unittest
@@ -760,6 +761,33 @@ def test_mode(self):
760761
finally:
761762
os.rmdir(dir)
762763

764+
@unittest.skipUnless(os.name == "nt", "Only on Windows.")
765+
def test_mode_win32(self):
766+
# Use icacls.exe to extract the users with some level of access
767+
# Main thing we are testing is that the BUILTIN\Users group has
768+
# no access. The exact ACL is going to vary based on which user
769+
# is running the test.
770+
dir = self.do_create()
771+
try:
772+
out = subprocess.check_output(["icacls.exe", dir], encoding="oem").casefold()
773+
finally:
774+
os.rmdir(dir)
775+
776+
dir = dir.casefold()
777+
users = set()
778+
found_user = False
779+
for line in out.strip().splitlines():
780+
acl = None
781+
# First line of result includes our directory
782+
if line.startswith(dir):
783+
acl = line[len(dir):].strip()
784+
elif line and line[:1].isspace():
785+
acl = line.strip()
786+
if acl:
787+
users.add(acl.partition(":")[0])
788+
789+
self.assertNotIn(r"BUILTIN\Users".casefold(), users)
790+
763791
def test_collision_with_existing_file(self):
764792
# mkdtemp tries another name when a file with
765793
# the chosen name already exists

‎Misc/NEWS.d/next/Security/2024-05-01-20-57-09.gh-issue-118486.K44KJG.rst

Copy file name to clipboard
+4Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
:func:`os.mkdir` on Windows now accepts *mode* of ``0o700`` to restrict
2+
the new directory to the current user. This fixes CVE-2024-4030
3+
affecting :func:`tempfile.mkdtemp` in scenarios where the base temporary
4+
directory is more permissive than the default.

‎Modules/posixmodule.c

Copy file name to clipboardExpand all lines: Modules/posixmodule.c
+41-3Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,12 @@
4040
#include "pycore_pystate.h" /* _PyRuntime */
4141
#include "pythread.h"
4242
#include "structmember.h"
43+
44+
#ifdef MS_WINDOWS
45+
# include <aclapi.h> // SetEntriesInAcl
46+
# include <sddl.h> // SDDL_REVISION_1
47+
#endif
48+
4349
#ifndef MS_WINDOWS
4450
# include "posixmodule.h"
4551
#else
@@ -4122,7 +4128,6 @@ os__path_splitroot_impl(PyObject *module, path_t *path)
41224128

41234129
#endif /* MS_WINDOWS */
41244130

4125-
41264131
/*[clinic input]
41274132
os.mkdir
41284133
@@ -4151,6 +4156,12 @@ os_mkdir_impl(PyObject *module, path_t *path, int mode, int dir_fd)
41514156
/*[clinic end generated code: output=a70446903abe821f input=e965f68377e9b1ce]*/
41524157
{
41534158
int result;
4159+
#ifdef MS_WINDOWS
4160+
int error = 0;
4161+
int pathError = 0;
4162+
SECURITY_ATTRIBUTES secAttr = { sizeof(secAttr) };
4163+
SECURITY_ATTRIBUTES *pSecAttr = NULL;
4164+
#endif
41544165

41554166
if (PySys_Audit("os.mkdir", "Oii", path->object, mode,
41564167
dir_fd == DEFAULT_DIR_FD ? -1 : dir_fd) < 0) {
@@ -4159,11 +4170,38 @@ os_mkdir_impl(PyObject *module, path_t *path, int mode, int dir_fd)
41594170

41604171
#ifdef MS_WINDOWS
41614172
Py_BEGIN_ALLOW_THREADS
4162-
result = CreateDirectoryW(path->wide, NULL);
4173+
if (mode == 0700 /* 0o700 */) {
4174+
ULONG sdSize;
4175+
pSecAttr = &secAttr;
4176+
// Set a discretionary ACL (D) that is protected (P) and includes
4177+
// inheritable (OICI) entries that allow (A) full control (FA) to
4178+
// SYSTEM (SY), Administrators (BA), and the owner (OW).
4179+
if (!ConvertStringSecurityDescriptorToSecurityDescriptorW(
4180+
L"D:P(A;OICI;FA;;;SY)(A;OICI;FA;;;BA)(A;OICI;FA;;;OW)",
4181+
SDDL_REVISION_1,
4182+
&secAttr.lpSecurityDescriptor,
4183+
&sdSize
4184+
)) {
4185+
error = GetLastError();
4186+
}
4187+
}
4188+
if (!error) {
4189+
result = CreateDirectoryW(path->wide, pSecAttr);
4190+
if (secAttr.lpSecurityDescriptor &&
4191+
// uncommonly, LocalFree returns non-zero on error, but still uses
4192+
// GetLastError() to see what the error code is
4193+
LocalFree(secAttr.lpSecurityDescriptor)) {
4194+
error = GetLastError();
4195+
}
4196+
}
41634197
Py_END_ALLOW_THREADS
41644198

4165-
if (!result)
4199+
if (error) {
4200+
return PyErr_SetFromWindowsErr(error);
4201+
}
4202+
if (!result) {
41664203
return path_error(path);
4204+
}
41674205
#else
41684206
Py_BEGIN_ALLOW_THREADS
41694207
#if HAVE_MKDIRAT

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.