diff --git a/emrg/sandbox/win32/abi.py b/emrg/sandbox/win32/abi.py index c25b3d11..6c2ff728 100644 --- a/emrg/sandbox/win32/abi.py +++ b/emrg/sandbox/win32/abi.py @@ -8,6 +8,8 @@ from __future__ import annotations +import ctypes + #: OpenProcess access required to query the current process token. PROCESS_QUERY_INFORMATION = 0x0400 #: Token right required by CreateProcessAsUserW. @@ -135,11 +137,38 @@ ) #: ``SECURITY_DESCRIPTOR_REVISION``: the only revision ``InitializeSecurityDescriptor`` accepts. SECURITY_DESCRIPTOR_REVISION = 1 +class SecurityDescriptor(ctypes.Structure): + """``SECURITY_DESCRIPTOR`` — the absolute form ``InitializeSecurityDescriptor`` builds. + + Declared here, beside the constant it sizes, so that constant can be + *measured*: a four-byte header (``Revision``, ``Sbz1``, ``Control``) followed + by four pointers is 20 bytes on a 32-bit interpreter and 40 on a 64-bit one, + and the DACL pointer — the last field the API writes — starts at offset 32 + there. A buffer sized by the 32-bit answer is overrun by half its length on a + 64-bit host, and because the allocation is a raw byte buffer rather than a + Python object's storage, nothing raises: the surplus write lands in the + process heap (issue #2509). + """ + + _fields_ = [ + ("Revision", ctypes.c_ubyte), + ("Sbz1", ctypes.c_ubyte), + ("Control", ctypes.c_ushort), + ("Owner", ctypes.c_void_p), + ("Group", ctypes.c_void_p), + ("Sacl", ctypes.c_void_p), + ("Dacl", ctypes.c_void_p), + ] + + #: The bytes an absolute ``SECURITY_DESCRIPTOR`` occupies. #: #: Named by the SDK for exactly this use — the buffer an absolute descriptor is -#: built in before ``MakeSelfRelativeSD`` measures the form the API wants. -SECURITY_DESCRIPTOR_MIN_LENGTH = 20 +#: built in before ``MakeSelfRelativeSD`` measures the form the API wants — and +#: the SDK defines it as ``sizeof(SECURITY_DESCRIPTOR)``, so it is taken from the +#: struct above and never written down. A literal ``20`` here is the 32-bit +#: answer: correct on one interpreter width and a 20-byte overrun on the other. +SECURITY_DESCRIPTOR_MIN_LENGTH = ctypes.sizeof(SecurityDescriptor) # --- process creation (Win32 process owner, win32-process/src/abi.ts) ------- diff --git a/tests/test_bash_v2_win32.py b/tests/test_bash_v2_win32.py index a28d34ed..5df8ab4b 100644 --- a/tests/test_bash_v2_win32.py +++ b/tests/test_bash_v2_win32.py @@ -76,6 +76,44 @@ SUB_AUTHORITY_CEILING = 2**30 - 1 +# ── the ABI declarations ───────────────────────────────────────────────── + + +def test_the_descriptor_buffer_holds_the_whole_struct_the_api_fills(): + """``SECURITY_DESCRIPTOR_MIN_LENGTH`` is a size, and it is measured, not chosen. + + Issue #2509. The constant was written down as ``20`` — the 32-bit answer — + while ``InitializeSecurityDescriptor`` fills the whole struct, whose DACL + pointer alone starts at offset 32 on a 64-bit interpreter. The 20-byte buffer + ``registry.apply_dacl`` handed the API was overrun by 20 bytes, the surplus + write landed in the process heap instead of in an object ctypes bounds, and + the crash surfaced in whichever test file the same xdist worker loaded next. + + This guard belongs on every platform, which is why it sits in this file's + cross-platform half: the Win32 *calls* need Windows, the struct's layout does + not, so the Linux and macOS legs — the ones that stay green — are exactly + where a hand-written size has to be caught. + """ + descriptor = abi.SecurityDescriptor + pointer = ctypes.sizeof(ctypes.c_void_p) + # The property that broke: the buffer is the size of the struct. + assert abi.SECURITY_DESCRIPTOR_MIN_LENGTH == ctypes.sizeof(descriptor) + # …and the one that still holds where the first is trivially true: the last + # field the API writes lives inside the buffer it was handed. + assert descriptor.Dacl.offset + pointer <= abi.SECURITY_DESCRIPTOR_MIN_LENGTH + # A truncated declaration would shrink both sides together, so pin the shape + # the SDK defines: one pointer-size field per member, in order. + assert [name for name, _ in descriptor._fields_] == [ + "Revision", + "Sbz1", + "Control", + "Owner", + "Group", + "Sacl", + "Dacl", + ] + + # ── capability SIDs ───────────────────────────────────────────────────────