[3.10] gh-118486: Support mkdir(mode=0o700) on Windows (GH-118488) (GH-118740)

Co-authored-by: Łukasz Langa <lukasz@langa.pl>

diff --git a/Doc/library/os.rst b/Doc/library/os.rst
index 28990c216af3d164707c163e624c0359ae47b1e1..8405fb1917e71442aa6d396269f58e2906cce48d 100644 (file)
--- a/Doc/library/os.rst
+++ b/Doc/library/os.rst
@@ -2065,6 +2065,10 @@ features:
    platform-dependent.  On some platforms, they are ignored and you should call
    :func:`chmod` explicitly to set them.
 
+   On Windows, a *mode* of ``0o700`` is specifically handled to apply access
+   control to the new directory such that only the current user and
+   administrators have access. Other values of *mode* are ignored.
+
    This function can also support :ref:`paths relative to directory descriptors
    <dir_fd>`.
 
@@ -2079,6 +2083,9 @@ features:
    .. versionchanged:: 3.6
       Accepts a :term:`path-like object`.
 
+   .. versionchanged:: 3.10.15
+      Windows now handles a *mode* of ``0o700``.
+
 
 .. function:: makedirs(name, mode=0o777, exist_ok=False)
 

diff --git a/Doc/whatsnew/3.10.rst b/Doc/whatsnew/3.10.rst
index d6ccf96a4cfec256eae02cc2ce523baa4fea39cc..f71a50163f49ea545df0d02e512138ac0f7e726a 100644 (file)
--- a/Doc/whatsnew/3.10.rst
+++ b/Doc/whatsnew/3.10.rst
@@ -1247,6 +1247,13 @@ Add :data:`~os.O_EVTONLY`, :data:`~os.O_FSYNC`, :data:`~os.O_SYMLINK`
 and :data:`~os.O_NOFOLLOW_ANY` for macOS.
 (Contributed by Dong-hee Na in :issue:`43106`.)
 
+As of 3.10.15, :func:`os.mkdir` and :func:`os.makedirs` on Windows now support
+passing a *mode* value of ``0o700`` to apply access control to the new
+directory. This implicitly affects :func:`tempfile.mkdtemp` and is a
+mitigation for CVE-2024-4030. Other values for *mode* continue to be
+ignored.
+(Contributed by Steve Dower in :gh:`118486`.)
+
 os.path
 -------
 
@@ -1399,6 +1406,14 @@ Add :data:`sys.stdlib_module_names`, containing the list of the standard library
 module names.
 (Contributed by Victor Stinner in :issue:`42955`.)
 
+tempfile
+--------
+
+As of 3.10.15 on Windows, the default mode ``0o700`` used by
+:func:`tempfile.mkdtemp` now limits access to the new directory due to
+changes to :func:`os.mkdir`. This is a mitigation for CVE-2024-4030.
+(Contributed by Steve Dower in :gh:`118486`.)
+
 _thread
 -------
 

diff --git a/Lib/test/test_os.py b/Lib/test/test_os.py
index 6b443c48f8fcb54618a680167bb5ba92b93bc212..044ce5c1a39f300e79d4ecfb0c328269ca02a402 100644 (file)
--- a/Lib/test/test_os.py
+++ b/Lib/test/test_os.py
@@ -1635,6 +1635,18 @@ class MakedirTests(unittest.TestCase):
         self.assertRaises(OSError, os.makedirs, path, exist_ok=True)
         os.remove(path)
 
+    @unittest.skipUnless(os.name == 'nt', "requires Windows")
+    def test_win32_mkdir_700(self):
+        base = os_helper.TESTFN
+        path = os.path.abspath(os.path.join(os_helper.TESTFN, 'dir'))
+        os.mkdir(path, mode=0o700)
+        out = subprocess.check_output(["cacls.exe", path, "/s"], encoding="oem")
+        os.rmdir(path)
+        self.assertEqual(
+            out.strip(),
+            f'{path} "D:P(A;OICI;FA;;;SY)(A;OICI;FA;;;BA)(A;OICI;FA;;;OW)"',
+        )
+
     def tearDown(self):
         path = os.path.join(os_helper.TESTFN, 'dir1', 'dir2', 'dir3',
                             'dir4', 'dir5', 'dir6')

diff --git a/Lib/test/test_tempfile.py b/Lib/test/test_tempfile.py
index 30d57baf977f520980f697c28749486641a7dae3..6e06bfc7f2f78da65204f24853eeca58d2cad460 100644 (file)
--- a/Lib/test/test_tempfile.py
+++ b/Lib/test/test_tempfile.py
@@ -11,6 +11,7 @@ import contextlib
 import stat
 import types
 import weakref
+import subprocess
 from unittest import mock
 
 import unittest
@@ -800,6 +801,33 @@ class TestMkdtemp(TestBadTempdir, BaseTestCase):
         finally:
             os.rmdir(dir)
 
+    @unittest.skipUnless(os.name == "nt", "Only on Windows.")
+    def test_mode_win32(self):
+        # Use icacls.exe to extract the users with some level of access
+        # Main thing we are testing is that the BUILTIN\Users group has
+        # no access. The exact ACL is going to vary based on which user
+        # is running the test.
+        dir = self.do_create()
+        try:
+            out = subprocess.check_output(["icacls.exe", dir], encoding="oem").casefold()
+        finally:
+            os.rmdir(dir)
+
+        dir = dir.casefold()
+        users = set()
+        found_user = False
+        for line in out.strip().splitlines():
+            acl = None
+            # First line of result includes our directory
+            if line.startswith(dir):
+                acl = line.removeprefix(dir).strip()
+            elif line and line[:1].isspace():
+                acl = line.strip()
+            if acl:
+                users.add(acl.partition(":")[0])
+
+        self.assertNotIn(r"BUILTIN\Users".casefold(), users)
+
     def test_collision_with_existing_file(self):
         # mkdtemp tries another name when a file with
         # the chosen name already exists

diff --git a/Misc/NEWS.d/next/Security/2024-05-01-20-57-09.gh-issue-118486.K44KJG.rst b/Misc/NEWS.d/next/Security/2024-05-01-20-57-09.gh-issue-118486.K44KJG.rst
new file mode 100644 (file)
index 0000000..a28a4e5
--- /dev/null
+++ b/Misc/NEWS.d/next/Security/2024-05-01-20-57-09.gh-issue-118486.K44KJG.rst
@@ -0,0 +1,4 @@
+:func:`os.mkdir` on Windows now accepts *mode* of ``0o700`` to restrict
+the new directory to the current user. This fixes CVE-2024-4030
+affecting :func:`tempfile.mkdtemp` in scenarios where the base temporary
+directory is more permissive than the default.

diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c
index c0421a94c17397bce33d5a0feaccc88b9885d6aa..43e69fc322b595d77187b730c684279eb9beefcc 100644 (file)
--- a/Modules/posixmodule.c
+++ b/Modules/posixmodule.c
@@ -33,6 +33,12 @@
 #include "pycore_import.h"        // _PyImport_ReInitLock()
 #include "pycore_initconfig.h"    // _PyStatus_EXCEPTION()
 #include "pycore_pystate.h"       // _PyInterpreterState_GET()
+
+#ifdef MS_WINDOWS
+#  include <aclapi.h>             // SetEntriesInAcl
+#  include <sddl.h>               // SDDL_REVISION_1
+#endif
+
 #include "structmember.h"         // PyMemberDef
 #ifndef MS_WINDOWS
 #  include "posixmodule.h"
@@ -4465,7 +4471,6 @@ os__path_splitroot_impl(PyObject *module, path_t *path)
 
 #endif /* MS_WINDOWS */
 
-
 /*[clinic input]
 os.mkdir
 
@@ -4495,6 +4500,12 @@ os_mkdir_impl(PyObject *module, path_t *path, int mode, int dir_fd)
 /*[clinic end generated code: output=a70446903abe821f input=a61722e1576fab03]*/
 {
     int result;
+#ifdef MS_WINDOWS
+    int error = 0;
+    int pathError = 0;
+    SECURITY_ATTRIBUTES secAttr = { sizeof(secAttr) };
+    SECURITY_ATTRIBUTES *pSecAttr = NULL;
+#endif
 #ifdef HAVE_MKDIRAT
     int mkdirat_unavailable = 0;
 #endif
@@ -4506,11 +4517,38 @@ os_mkdir_impl(PyObject *module, path_t *path, int mode, int dir_fd)
 
 #ifdef MS_WINDOWS
     Py_BEGIN_ALLOW_THREADS
-    result = CreateDirectoryW(path->wide, NULL);
+    if (mode == 0700 /* 0o700 */) {
+        ULONG sdSize;
+        pSecAttr = &secAttr;
+        // Set a discretionary ACL (D) that is protected (P) and includes
+        // inheritable (OICI) entries that allow (A) full control (FA) to
+        // SYSTEM (SY), Administrators (BA), and the owner (OW).
+        if (!ConvertStringSecurityDescriptorToSecurityDescriptorW(
+            L"D:P(A;OICI;FA;;;SY)(A;OICI;FA;;;BA)(A;OICI;FA;;;OW)",
+            SDDL_REVISION_1,
+            &secAttr.lpSecurityDescriptor,
+            &sdSize
+        )) {
+            error = GetLastError();
+        }
+    }
+    if (!error) {
+        result = CreateDirectoryW(path->wide, pSecAttr);
+        if (secAttr.lpSecurityDescriptor &&
+            // uncommonly, LocalFree returns non-zero on error, but still uses
+            // GetLastError() to see what the error code is
+            LocalFree(secAttr.lpSecurityDescriptor)) {
+            error = GetLastError();
+        }
+    }
     Py_END_ALLOW_THREADS
 
-    if (!result)
+    if (error) {
+        return PyErr_SetFromWindowsErr(error);
+    }
+    if (!result) {
         return path_error(path);
+    }
 #else
     Py_BEGIN_ALLOW_THREADS
 #if HAVE_MKDIRAT