Repository navigation
[2.7.18.16] Support mkdir(mode=0700) on Windows (CVE-2024-4030) - #108
Conversation
Backport of python/cpython pythongh-118486 (eb29e2f; 3.10: c8f868d) to 2.7.18.x. On Windows os.mkdir() ignored its mode, so tempfile.mkdtemp(), which calls os.mkdir(name, 0700), created directories that inherit the parent's ACL. Under a shared parent other local users can read the "private" temporary directory. posix_mkdir() now handles mode 0700 on Windows by creating the directory with a security descriptor whose protected discretionary ACL only grants full control (inherited by files and subdirectories) to SYSTEM, Administrators and the owner: "D:P(A;OICI;FA;;;SY)(A;OICI;FA;;;BA) (A;OICI;FA;;;OW)". Other modes are ignored, as before. The same mode check on POSIX is unchanged. 2.7.18.x: 2.7's posix_mkdir() has a wide (CreateDirectoryW) and a narrow (CreateDirectoryA) path, and mkdtemp() normally takes the narrow one for str names, so the descriptor is built by a shared helper and applied on both. C89 declarations. ConvertStringSecurityDescriptorToSecurityDescriptorW is in advapi32, which pythoncore already links (_winreg). The docstring and Doc/library/os.rst describe the 0700 handling as upstream does. Tests (Windows only, ported from upstream): test_os.MakedirTests. test_win32_mkdir_700 checks the SDDL with cacls.exe, and test_tempfile.test_mkdtemp.test_mode_win32 checks with icacls.exe that BUILTIN\Users has no access. Not built or run on Windows yet: the Windows code was syntax-checked with gcc -std=c89 -pedantic against stub declarations, and the Linux build and test_os/test_tempfile pass. Co-Authored-By: Claude Opus 5.5 <[email protected]>
ezequielp-activestate
left a comment
ezequielp-activestate
left a comment
There was a problem hiding this comment.
No blockers. Approving, with one caveat: nothing here was run on Windows.
Matches upstream. The Windows code is upstream's final form of the CVE-2024-4030 fix:
- the original pythongh-118486 change (81939da);
- the follow-up that switched to OWNER RIGHTS (8ed5466);
- pythongh-118773, which replaced the ACL-building code with the language-invariant SDDL string
D:P(A;OICI;FA;;;SY)(A;OICI;FA;;;BA)(A;OICI;FA;;;OW)(8af84b5).
The test is the exact-ACL version from 94591dc. Upstream main, 3.14, 3.13 and 3.12 have had no later change to this code. The mode == 0700 check, the "convert, then CreateDirectory, then LocalFree" order, and the error precedence (PyErr_SetFromWindowsErr for a descriptor or free error, the usual win32_error* otherwise) are the same as upstream's os_mkdir_impl. Applying it to both 2.7 paths, CreateDirectoryW for unicode and CreateDirectoryA for the str path that tempfile.mkdtemp() uses, is the right adaptation. Other modes still pass a NULL descriptor, exactly as before.
Checks:
- The two helpers, with a
posix_mkdir-shaped caller, compile with MinGW-w64's real Windows headers under-std=c89 -Wall -Wextra -pedanticwith no warnings, and link with-ladvapi32. - pythoncore gets advapi32 from the inherited default
%(AdditionalDependencies)(PCbuild/pythoncore.vcxproj:76), the same way_winreg'sReg*calls resolve. So no project change is needed. - Linux build of this head: no new warnings in
posixmodule.c.test_os(78 tests) andtest_tempfile(67) pass, with the two new Windows tests skipped. Base: 77 and 66, all passing.
⚪ #include <sddl.h> sits inside the #ifdef _MSC_VER block (Modules/posixmodule.c:281), while the helpers are under MS_WINDOWS. That's fine for the MSVC builds 2.7 supports, but a non-MSVC Windows build would get an implicit declaration of ConvertStringSecurityDescriptorToSecurityDescriptorW. Upstream includes it under MS_WINDOWS. Optional.
Not assessed: a Windows build, the two new Windows tests (cacls.exe / icacls.exe), and the behaviour on network shares or FAT volumes, where CreateDirectory with a security descriptor behaves as upstream's does.
|
Correction: this PR's commit message cites |
Bump PY_VERSION to 2.7.18.16 and add release notes for the security fixes merged into 2.7.18.16 (#92-#108, #111): tarfile (CVE-2026-11972), csv.Sniffer (CVE-2026-18503), ftplib.ftpcp (CVE-2026-8328), httplib (CVE-2026-103995, CVE-2021-3737 and GHSA-w4q2-g22w-6fr4), SimpleHTTPServer (CVE-2021-28861), mimetypes on Windows (CVE-2024-3220), pyexpat (CVE-2026-4224), bz2 (CVE-2026-9669), ElementPath (CVE-2026-6879), urllib2 (CVE-2026-15806), HTMLParser (CVE-2026-15308), bundled Expat 2.9.0 (30 CVEs), the Expat hash salt (CVE-2026-7210), the ssl SNI callback (CVE-2026-19445), unicodedata (CVE-2026-3276) and os.mkdir on Windows (CVE-2024-4030), plus the review follow-up build fixes. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Backport of upstream pythongh-118486 (CVE-2024-4030): on Windows,
os.mkdir(path, 0700)and thereforetempfile.mkdtemp()created directories readable by other users. Part of 2.7.18.16 (Linear PROD-1588).Problem
On Windows,
os.mkdir()ignored its mode.tempfile.mkdtemp()callsos.mkdir(name, 0700), so its "private" directory inherited the parent's ACL, and any local user with access to the parent could read it.Fix (same as upstream)
For mode
0700,posix_mkdir()creates the directory with the security descriptorD:P(A;OICI;FA;;;SY)(A;OICI;FA;;;BA)(A;OICI;FA;;;OW). That's a protected ACL granting full control, inherited by children, only to SYSTEM, Administrators and the owner. Other modes are still ignored on Windows. Nothing changes on POSIX.Behavior change (same as upstream): on Windows, a directory created with
os.mkdir(p, 0700)or bytempfile.mkdtemp()is no longer accessible to other non-admin users. That includes services or other accounts that previously relied on the inherited ACL.2.7 adaptation:
posix_mkdir()has both aCreateDirectoryWpath (unicode) and aCreateDirectoryApath (str, which is whatmkdtemp()normally uses). A shared helper builds the descriptor and both paths apply it.ConvertStringSecurityDescriptorToSecurityDescriptorWis in advapi32, which pythoncore already links through_winreg, so there's no project change.Doc/library/os.rstgain upstream's note.Testing
test_os.MakedirTests.test_win32_mkdir_700checks the exact SDDL withcacls.exe.test_tempfile.test_mkdtemp.test_mode_win32checks withicacls.exethatBUILTIN\Usershas no access.posix_mkdir) compiles cleanly undergcc -std=c89 -Wall -Wextra -pedantic -fsyntax-onlyagainst stub Win32 declarations.