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 eb29e2f

Browse filesBrowse files
zoobazooba
authored
gh-118486: Support mkdir(mode=0o700) on Windows (GH-118488)
1 parent c0d257c commit eb29e2f
Copy full SHA for eb29e2f

File tree

6 files changed

+106
-2
lines changed
Filter options

6 files changed

+106
-2
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
@@ -2356,6 +2356,10 @@ features:
23562356
platform-dependent. On some platforms, they are ignored and you should call
23572357
:func:`chmod` explicitly to set them.
23582358

2359+
On Windows, a *mode* of ``0o700`` is specifically handled to apply access
2360+
control to the new directory such that only the current user and
2361+
administrators have access. Other values of *mode* are ignored.
2362+
23592363
This function can also support :ref:`paths relative to directory descriptors
23602364
<dir_fd>`.
23612365

@@ -2370,6 +2374,9 @@ features:
23702374
.. versionchanged:: 3.6
23712375
Accepts a :term:`path-like object`.
23722376

2377+
.. versionchanged:: 3.12.4
2378+
Windows now handles a *mode* of ``0o700``.
2379+
23732380

23742381
.. function:: makedirs(name, mode=0o777, exist_ok=False)
23752382

‎Doc/whatsnew/3.12.rst

Copy file name to clipboardExpand all lines: Doc/whatsnew/3.12.rst
+11Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -778,6 +778,13 @@ os
778778
Both functions may be significantly faster on newer releases of
779779
Windows. (Contributed by Steve Dower in :gh:`99726`.)
780780

781+
* As of 3.12.4, :func:`os.mkdir` and :func:`os.makedirs` on Windows
782+
now support passing a *mode* value of ``0o700`` to apply access
783+
control to the new directory. This implicitly affects
784+
:func:`tempfile.mkdtemp` and is a mitigation for :cve:`2024-4030`.
785+
Other values for *mode* continue to be ignored.
786+
(Contributed by Steve Dower in :gh:`118486`.)
787+
781788
os.path
782789
-------
783790

@@ -925,6 +932,10 @@ tempfile
925932
*delete_on_close* (Contributed by Evgeny Zorin in :gh:`58451`.)
926933
* :func:`tempfile.mkdtemp` now always returns an absolute path, even if the
927934
argument provided to the *dir* parameter is a relative path.
935+
* As of 3.12.4 on Windows, the default mode ``0o700`` used by
936+
:func:`tempfile.mkdtemp` now limits access to the new directory due to
937+
changes to :func:`os.mkdir`. This is a mitigation for :cve:`2024-4030`.
938+
(Contributed by Steve Dower in :gh:`118486`.)
928939

929940
threading
930941
---------

‎Lib/test/test_os.py

Copy file name to clipboardExpand all lines: Lib/test/test_os.py
+19Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1797,6 +1797,25 @@ def test_exist_ok_existing_regular_file(self):
17971797
self.assertRaises(OSError, os.makedirs, path, exist_ok=True)
17981798
os.remove(path)
17991799

1800+
@unittest.skipUnless(os.name == 'nt', "requires Windows")
1801+
def test_win32_mkdir_700(self):
1802+
base = os_helper.TESTFN
1803+
path1 = os.path.join(os_helper.TESTFN, 'dir1')
1804+
path2 = os.path.join(os_helper.TESTFN, 'dir2')
1805+
# mode=0o700 is special-cased to override ACLs on Windows
1806+
# There's no way to know exactly how the ACLs will look, so we'll
1807+
# check that they are different from a regularly created directory.
1808+
os.mkdir(path1, mode=0o700)
1809+
os.mkdir(path2, mode=0o777)
1810+
1811+
out1 = subprocess.check_output(["icacls.exe", path1], encoding="oem")
1812+
out2 = subprocess.check_output(["icacls.exe", path2], encoding="oem")
1813+
os.rmdir(path1)
1814+
os.rmdir(path2)
1815+
out1 = out1.replace(path1, "<PATH>")
1816+
out2 = out2.replace(path2, "<PATH>")
1817+
self.assertNotEqual(out1, out2)
1818+
18001819
def tearDown(self):
18011820
path = os.path.join(os_helper.TESTFN, 'dir1', 'dir2', 'dir3',
18021821
'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
@@ -13,6 +13,7 @@
1313
import weakref
1414
import gc
1515
import shutil
16+
import subprocess
1617
from unittest import mock
1718

1819
import unittest
@@ -803,6 +804,33 @@ def test_mode(self):
803804
finally:
804805
os.rmdir(dir)
805806

807+
@unittest.skipUnless(os.name == "nt", "Only on Windows.")
808+
def test_mode_win32(self):
809+
# Use icacls.exe to extract the users with some level of access
810+
# Main thing we are testing is that the BUILTIN\Users group has
811+
# no access. The exact ACL is going to vary based on which user
812+
# is running the test.
813+
dir = self.do_create()
814+
try:
815+
out = subprocess.check_output(["icacls.exe", dir], encoding="oem").casefold()
816+
finally:
817+
os.rmdir(dir)
818+
819+
dir = dir.casefold()
820+
users = set()
821+
found_user = False
822+
for line in out.strip().splitlines():
823+
acl = None
824+
# First line of result includes our directory
825+
if line.startswith(dir):
826+
acl = line.removeprefix(dir).strip()
827+
elif line and line[:1].isspace():
828+
acl = line.strip()
829+
if acl:
830+
users.add(acl.partition(":")[0])
831+
832+
self.assertNotIn(r"BUILTIN\Users".casefold(), users)
833+
806834
def test_collision_with_existing_file(self):
807835
# mkdtemp tries another name when a file with
808836
# 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
+37-2Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@
3232
# include <winioctl.h>
3333
# include <lmcons.h> // UNLEN
3434
# include "osdefs.h" // SEP
35+
# include <aclapi.h> // SetEntriesInAcl
36+
# include <sddl.h> // SDDL_REVISION_1
3537
# if defined(MS_WINDOWS_DESKTOP) || defined(MS_WINDOWS_SYSTEM)
3638
# define HAVE_SYMLINK
3739
# endif /* MS_WINDOWS_DESKTOP | MS_WINDOWS_SYSTEM */
@@ -5342,6 +5344,12 @@ os_mkdir_impl(PyObject *module, path_t *path, int mode, int dir_fd)
53425344
/*[clinic end generated code: output=a70446903abe821f input=a61722e1576fab03]*/
53435345
{
53445346
int result;
5347+
#ifdef MS_WINDOWS
5348+
int error = 0;
5349+
int pathError = 0;
5350+
SECURITY_ATTRIBUTES secAttr = { sizeof(secAttr) };
5351+
SECURITY_ATTRIBUTES *pSecAttr = NULL;
5352+
#endif
53455353
#ifdef HAVE_MKDIRAT
53465354
int mkdirat_unavailable = 0;
53475355
#endif
@@ -5353,11 +5361,38 @@ os_mkdir_impl(PyObject *module, path_t *path, int mode, int dir_fd)
53535361

53545362
#ifdef MS_WINDOWS
53555363
Py_BEGIN_ALLOW_THREADS
5356-
result = CreateDirectoryW(path->wide, NULL);
5364+
if (mode == 0700 /* 0o700 */) {
5365+
ULONG sdSize;
5366+
pSecAttr = &secAttr;
5367+
// Set a discreationary ACL (D) that is protected (P) and includes
5368+
// inheritable (OICI) entries that allow (A) full control (FA) to
5369+
// SYSTEM (SY), Administrators (BA), and the owner (OW).
5370+
if (!ConvertStringSecurityDescriptorToSecurityDescriptorW(
5371+
L"D:P(A;OICI;FA;;;SY)(A;OICI;FA;;;BA)(A;OICI;FA;;;OW)",
5372+
SDDL_REVISION_1,
5373+
&secAttr.lpSecurityDescriptor,
5374+
&sdSize
5375+
)) {
5376+
error = GetLastError();
5377+
}
5378+
}
5379+
if (!error) {
5380+
result = CreateDirectoryW(path->wide, pSecAttr);
5381+
if (secAttr.lpSecurityDescriptor &&
5382+
// uncommonly, LocalFree returns non-zero on error, but still uses
5383+
// GetLastError() to see what the error code is
5384+
LocalFree(secAttr.lpSecurityDescriptor)) {
5385+
error = GetLastError();
5386+
}
5387+
}
53575388
Py_END_ALLOW_THREADS
53585389

5359-
if (!result)
5390+
if (error) {
5391+
return PyErr_SetFromWindowsErr(error);
5392+
}
5393+
if (!result) {
53605394
return path_error(path);
5395+
}
53615396
#else
53625397
Py_BEGIN_ALLOW_THREADS
53635398
#if HAVE_MKDIRAT

0 commit comments

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