FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

[3.9] gh-118486: Support mkdir(mode=0o700) on Windows (GH-118488) (GH… · python/cpython@5130731 · GitHub

/ cpython Public

Commit 5130731

Browse files
zooba
and
ambv
authored
[3.9] gh-118486: Support mkdir(mode=0o700) on Windows (GH-118488) (GH-118741)
Co-authored-by: Łukasz Langa <lukasz@langa.pl>
1 parent b228655 commit 5130731

6 files changed

Lines changed: 107 additions & 3 deletions

File tree

‎Doc/library/os.rst‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1929,6 +1929,10 @@ features:
19291929
platform-dependent. On some platforms, they are ignored and you should call
19301930
:func:`chmod` explicitly to set them.
19311931

1932+
On Windows, a *mode* of ``0o700`` is specifically handled to apply access
1933+
control to the new directory such that only the current user and
1934+
administrators have access. Other values of *mode* are ignored.
1935+
19321936
This function can also support :ref:`paths relative to directory descriptors
19331937
<dir_fd>`.
19341938

@@ -1943,6 +1947,9 @@ features:
19431947
.. versionchanged:: 3.6
19441948
Accepts a :term:`path-like object`.
19451949

1950+
.. versionchanged:: 3.9.20
1951+
Windows now handles a *mode* of ``0o700``.
1952+
19461953

19471954
.. function:: makedirs(name, mode=0o777, exist_ok=False)
19481955

‎Doc/whatsnew/3.9.rst‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -613,6 +613,13 @@ Added :func:`os.waitstatus_to_exitcode` function:
613613
convert a wait status to an exit code.
614614
(Contributed by Victor Stinner in :issue:`40094`.)
615615

616+
As of 3.9.20, :func:`os.mkdir` and :func:`os.makedirs` on Windows now support
617+
passing a *mode* value of ``0o700`` to apply access control to the new
618+
directory. This implicitly affects :func:`tempfile.mkdtemp` and is a
619+
mitigation for CVE-2024-4030. Other values for *mode* continue to be
620+
ignored.
621+
(Contributed by Steve Dower in :gh:`118486`.)
622+
616623
pathlib
617624
-------
618625

@@ -704,6 +711,14 @@ Previously, :attr:`sys.stderr` was block-buffered when non-interactive. Now
704711
``stderr`` defaults to always being line-buffered.
705712
(Contributed by Jendrik Seipp in :issue:`13601`.)
706713

714+
tempfile
715+
--------
716+
717+
As of 3.9.20 on Windows, the default mode ``0o700`` used by
718+
:func:`tempfile.mkdtemp` now limits access to the new directory due to
719+
changes to :func:`os.mkdir`. This is a mitigation for CVE-2024-4030.
720+
(Contributed by Steve Dower in :gh:`118486`.)
721+
707722
tracemalloc
708723
-----------
709724

‎Lib/test/test_os.py‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1493,6 +1493,18 @@ def test_exist_ok_existing_regular_file(self):
14931493
self.assertRaises(OSError, os.makedirs, path, exist_ok=True)
14941494
os.remove(path)
14951495

1496+
@unittest.skipUnless(os.name == 'nt', "requires Windows")
1497+
def test_win32_mkdir_700(self):
1498+
base = support.TESTFN
1499+
path = os.path.abspath(os.path.join(support.TESTFN, 'dir'))
1500+
os.mkdir(path, mode=0o700)
1501+
out = subprocess.check_output(["cacls.exe", path, "/s"], encoding="oem")
1502+
os.rmdir(path)
1503+
self.assertEqual(
1504+
out.strip(),
1505+
f'{path} "D:P(A;OICI;FA;;;SY)(A;OICI;FA;;;BA)(A;OICI;FA;;;OW)"',
1506+
)
1507+
14961508
def tearDown(self):
14971509
path = os.path.join(support.TESTFN, 'dir1', 'dir2', 'dir3',
14981510
'dir4', 'dir5', 'dir6')

‎Lib/test/test_tempfile.py‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import stat
1212
import types
1313
import weakref
14+
import subprocess
1415
from unittest import mock
1516

1617
import unittest
@@ -772,6 +773,33 @@ def test_mode(self):
772773
finally:
773774
os.rmdir(dir)
774775

776+
@unittest.skipUnless(os.name == "nt", "Only on Windows.")
777+
def test_mode_win32(self):
778+
# Use icacls.exe to extract the users with some level of access
779+
# Main thing we are testing is that the BUILTIN\Users group has
780+
# no access. The exact ACL is going to vary based on which user
781+
# is running the test.
782+
dir = self.do_create()
783+
try:
784+
out = subprocess.check_output(["icacls.exe", dir], encoding="oem").casefold()
785+
finally:
786+
os.rmdir(dir)
787+
788+
dir = dir.casefold()
789+
users = set()
790+
found_user = False
791+
for line in out.strip().splitlines():
792+
acl = None
793+
# First line of result includes our directory
794+
if line.startswith(dir):
795+
acl = line.removeprefix(dir).strip()
796+
elif line and line[:1].isspace():
797+
acl = line.strip()
798+
if acl:
799+
users.add(acl.partition(":")[0])
800+
801+
self.assertNotIn(r"BUILTIN\Users".casefold(), users)
802+
775803
def test_collision_with_existing_file(self):
776804
# mkdtemp tries another name when a file with
777805
# the chosen name already exists

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

Lines 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‎

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,12 @@
2424
#include "pycore_ceval.h" // _PyEval_ReInitThreads()
2525
#include "pycore_import.h" // _PyImport_ReInitLock()
2626
#include "pycore_pystate.h" // _PyInterpreterState_GET()
27+
28+
#ifdef MS_WINDOWS
29+
# include <aclapi.h> // SetEntriesInAcl
30+
# include <sddl.h> // SDDL_REVISION_1
31+
#endif
32+
2733
#include "structmember.h" // PyMemberDef
2834
#ifndef MS_WINDOWS
2935
# include "posixmodule.h"
@@ -4425,7 +4431,6 @@ os__path_splitroot_impl(PyObject *module, path_t *path)
44254431

44264432
#endif /* MS_WINDOWS */
44274433

4428-
44294434
/*[clinic input]
44304435
os.mkdir
44314436
@@ -4454,6 +4459,12 @@ os_mkdir_impl(PyObject *module, path_t *path, int mode, int dir_fd)
44544459
/*[clinic end generated code: output=a70446903abe821f input=e965f68377e9b1ce]*/
44554460
{
44564461
int result;
4462+
#ifdef MS_WINDOWS
4463+
int error = 0;
4464+
int pathError = 0;
4465+
SECURITY_ATTRIBUTES secAttr = { sizeof(secAttr) };
4466+
SECURITY_ATTRIBUTES *pSecAttr = NULL;
4467+
#endif
44574468
#ifdef HAVE_MKDIRAT
44584469
int mkdirat_unavailable = 0;
44594470
#endif
@@ -4465,11 +4476,38 @@ os_mkdir_impl(PyObject *module, path_t *path, int mode, int dir_fd)
44654476

44664477
#ifdef MS_WINDOWS
44674478
Py_BEGIN_ALLOW_THREADS
4468-
result = CreateDirectoryW(path->wide, NULL);
4479+
if (mode == 0700 /* 0o700 */) {
4480+
ULONG sdSize;
4481+
pSecAttr = &secAttr;
4482+
// Set a discretionary ACL (D) that is protected (P) and includes
4483+
// inheritable (OICI) entries that allow (A) full control (FA) to
4484+
// SYSTEM (SY), Administrators (BA), and the owner (OW).
4485+
if (!ConvertStringSecurityDescriptorToSecurityDescriptorW(
4486+
L"D:P(A;OICI;FA;;;SY)(A;OICI;FA;;;BA)(A;OICI;FA;;;OW)",
4487+
SDDL_REVISION_1,
4488+
&secAttr.lpSecurityDescriptor,
4489+
&sdSize
4490+
)) {
4491+
error = GetLastError();
4492+
}
4493+
}
4494+
if (!error) {
4495+
result = CreateDirectoryW(path->wide, pSecAttr);
4496+
if (secAttr.lpSecurityDescriptor &&
4497+
// uncommonly, LocalFree returns non-zero on error, but still uses
4498+
// GetLastError() to see what the error code is
4499+
LocalFree(secAttr.lpSecurityDescriptor)) {
4500+
error = GetLastError();
4501+
}
4502+
}
44694503
Py_END_ALLOW_THREADS
44704504

4471-
if (!result)
4505+
if (error) {
4506+
return PyErr_SetFromWindowsErr(error);
4507+
}
4508+
if (!result) {
44724509
return path_error(path);
4510+
}
44734511
#else
44744512
Py_BEGIN_ALLOW_THREADS
44754513
#if HAVE_MKDIRAT

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL