Unchecked count multiplications in the System Informer phsvc helper

When System Informer runs unelevated and an action needs administrator rights, such as reconfiguring a service, terminating a process or unloading a driver, it can prompt for elevation (a setting decides whether it asks, always elevates or never does) and start a second copy of itself with -phsvc. That copy is phsvc: it creates an LPC port and carries out API calls on behalf of the UI. PR #2879 put checks on six sizes computed as a count times sizeof: the three array lengths in phsvc’s ChangeServiceConfig2 handler, the reply length in its heap-information API, and the allocation sized from the same heap count in phlib/native.c and SystemInformer/memprv.c.

Before the arithmetic, the plumbing. Whether any of this matters depends on who can reach that port and what they could already do there.

The phsvc channel

PhUiConnectToPhSvcEx in actions.c tries to connect to the port first. If that fails and the user accepts the elevation prompt, it starts the helper:

status = PhShellProcessHacker(
    WindowHandle,
    L"-phsvc",
    SW_HIDE,
    PH_SHELL_EXECUTE_ADMIN,
    0,
    0,
    NULL
    );

then retries the connection up to ten times, a second apart. In the elevated copy, main.c sees -phsvc and calls PhSvcMain, which creates \BaseNamedObjects\SiSvcApiPort and waits. It exits 30 seconds after its last client disconnects.

The port gets this DACL:

PhAddAccessAllowedAce(dacl, ACL_REVISION, PORT_ALL_ACCESS, administratorsSid);
PhAddAccessAllowedAce(dacl, ACL_REVISION, PORT_CONNECT, (PSID)&PhSeEveryoneSid);

To connect, the client (PhSvcConnectToServer in clapi.c) creates an 8 MB section and passes it to NtConnectPort as its port view. It then builds a heap on that view with RtlCreateHeap and allocates every argument string and buffer from it. An argument travels as a PH_RELATIVE_STRINGREF, a ULONG Length and a ULONG Offset from the start of the view, so the LPC message itself holds only the API number and fixed-size fields.

On the server, two threads loop on NtReplyWaitReceivePort. Connection requests go to PhSvcHandleConnectionRequest, and requests go to a table:

if (
    Payload->ApiNumber == 0 ||
    (ULONG)Payload->ApiNumber >= (ULONG)PhSvcMaximumApiNumber ||
    !PhSvcApiCallTable[Payload->ApiNumber - 1]
    )
{
    Payload->ReturnStatus = STATUS_INVALID_SYSTEM_SERVICE;
    *ReplyPortHandle = Client->PortHandle;
    return;
}

status = PhSvcApiCallTable[Payload->ApiNumber - 1](Client, Payload);

There is no per-call check after the connection is accepted. The table has 20 entries, among them ExecuteRunAsCommand, CreateService with a caller-supplied BinaryPathName, ChangeServiceConfig, SetServiceSecurity, CreateProcessIgnoreIfeoDebugger, ControlProcess and UnloadDriver. ChangeServiceConfig2 is number 8.

For a flat argument, PhSvcValidateString adds the offset to the client’s view base and checks the result against the view’s limit and the required alignment. PhSvcCaptureBuffer then copies the bytes into a fresh heap allocation, so everything below parses a private copy the client can no longer change.

Pointer packing

ChangeServiceConfig2 takes an info level and a pointer to a level-specific struct, and two of those structs contain pointers to arrays. SERVICE_FAILURE_ACTIONS has lpsaActions[cActions]. SERVICE_TRIGGER_INFO has pTriggers[cTriggers], each SERVICE_TRIGGER has pDataItems[cDataItems], and each data item has pData[cbData]. A pointer means nothing in another process, so the client flattens the struct into one blob. PhSvcpPackRoot copies the root struct, and PhSvcpPackBuffer appends each pointed-to buffer, aligned, and overwrites the pointer field in the copy with that buffer’s offset in the blob:

PhAppendBytesBuilderEx(BytesBuilder, *PointerInBytesBuilder, Length, Alignment, &offset);
newBase = (ULONG_PTR)BytesBuilder->Bytes->Buffer;

PointerInBytesBuilder = (PVOID *)((ULONG_PTR)PointerInBytesBuilder - oldBase + newBase);
*PointerInBytesBuilder = (PVOID)offset;

The server reverses it on the captured copy. PhSvcpUnpackRoot checks that the blob is at least as large as the root struct. PhSvcpUnpackBuffer takes a pointer field that holds an offset, checks it, and replaces it with a real pointer into the copy:

offset = (SIZE_T)*OffsetInBuffer;

if (offset == 0)
{
    if (AllowNull)
        return STATUS_SUCCESS;
    else
        return STATUS_ACCESS_VIOLATION;
}

if (offset + Length < offset)
    return STATUS_ACCESS_VIOLATION;
if (offset + Length > PackedData->Length)
    return STATUS_ACCESS_VIOLATION;
if (offset & (Alignment - 1))
    return STATUS_DATATYPE_MISALIGNMENT;

*OffsetInBuffer = PTR_ADD_OFFSET(CapturedBuffer, offset);

This function is careful, down to checking offset + Length for wraparound. Length itself comes from the caller, and the function has to take it as given.

The unchecked count

This is the SERVICE_CONFIG_FAILURE_ACTIONS case before the patch:

if (!NT_SUCCESS(status = PhSvcpUnpackBuffer(&packedData, info, &failureActions->lpsaActions, failureActions->cActions * sizeof(SC_ACTION), __alignof(SC_ACTION), TRUE)))
    goto CleanupExit;

if (failureActions->lpsaActions)
{
    ULONG i;

    for (i = 0; i < failureActions->cActions; i++)
    {
        if (failureActions->lpsaActions[i].Type == SC_ACTION_RESTART)
        {
            desiredAccess |= SERVICE_START;
            break;
        }
    }
}

failureActions points at the root struct inside the captured blob, so cActions is a value the client wrote. The same holds for cTriggers and for every cDataItems. The validated length is the count times the element size, and the loop then walks the count. If the product comes out smaller than the count’s worth of elements, the bounds check passes and the loop runs past what was checked.

Multiplication overflow

The counts are DWORD, 32 bits on every Windows build. sizeof yields size_t, and C converts both operands to a common type before it multiplies. In a 64-bit build size_t is 64 bits, the count is widened first, and the product can’t overflow. The largest possible one, 0xFFFFFFFF times 32 for SERVICE_TRIGGER, is about 128 GiB, and PhSvcpUnpackBuffer rejects it because the blob length is a ULONG. In a 32-bit build size_t is 32 bits and the product is taken modulo 2^32.

So the PR description’s line about malformed input forcing wraparound holds for a 32-bit build only. I checked the arithmetic with a small C program: native on x86-64 for the 64-bit layouts, and with the x86 layout emulated (32-bit pointers and size_t) for the other column.

Each x86 product is exactly one element. Take the first row: 0x20000001 * 8 is 0x100000008, a 33-bit value. A 32-bit size_t keeps the low 32 bits, 0x00000008.

A blob that uses it, for a 32-bit helper and SERVICE_CONFIG_FAILURE_ACTIONS: the 20-byte root at offset 0 with cActions = 0x20000001 and lpsaActions = 0x14, then one 8-byte SC_ACTION at 0x14, for a total length of 0x1c. PhSvcpUnpackBuffer computes 0x14 + 8 = 0x1c, which isn’t greater than the blob length, and 0x14 meets the 4-byte alignment of SC_ACTION, so it accepts. The loop then reads lpsaActions[1] at offset 0x1c, the first byte past the captured allocation, and continues in 8-byte steps until it finds a Type equal to SC_ACTION_RESTART or the read faults. If it stops early, the struct still claims 536,870,913 actions when it goes on to PhChangeServiceConfig2.

The trigger case is worse, because PhSvcpUnpackBuffer writes:

for (i = 0; i < triggerInfo->cTriggers; i++)
{
    trigger = &triggerInfo->pTriggers[i];

    if (!NT_SUCCESS(status = PhSvcpUnpackBuffer(&packedData, info, &trigger->pTriggerSubtype, sizeof(GUID), __alignof(GUID), TRUE)))
        goto CleanupExit;

With cTriggers wrapped to one element, trigger for i = 1 points past the captured buffer, and PhSvcpUnpackBuffer reads the out-of-range pTriggerSubtype as an offset. A zero there counts as NULL and the loop moves on. A value that passes the bounds and alignment checks gets overwritten with a pointer into the captured blob. Anything else ends the call with an error. Past the end of the allocation, the loop is an out-of-bounds read with a conditional out-of-bounds write of a pointer-sized value, and whatever heap memory follows the blob decides what is written where. pDataItems and the cDataItems loop under it behave the same way.

That is what the code does. I didn’t run it. I didn’t build a 32-bit client, send a crafted blob, or crash a helper. Nothing in this section was demonstrated.

32-bit builds

The project’s build tool builds 32-bit binaries as part of a release (its Release flag set is Build32bit | Build64bit | BuildArm64bit | BuildRelease | BuildApi | BuildVerbose). On 64-bit Windows, the 64-bit UI uses a 32-bit phsvc for two jobs on WOW64 processes: minidumps (mdump.c) and heap information (heapinfo.c). Both connect the same way:

#ifdef _WIN64
    if (Context->ProcessItem->IsWow64Process)
    {
        if (PhUiConnectToPhSvcEx(Context->WindowHandle, Wow64PhSvcMode, FALSE))
        {

For that mode, PhpStartPhSvcProcess looks for a copy of its own executable in an x86\ folder next to it and starts it with -phsvc and PH_SHELL_EXECUTE_DEFAULT, without the admin verb, so the helper gets the token of the UI that started it. It listens on its own port, \BaseNamedObjects\SiSvcWow64ApiPort. The -phsvc branch in main.c runs before the check that refuses to start the 32-bit build under WOW64 in non-debug builds, which is how this helper starts at all.

The 32-bit helper uses the same dispatch table. phsvcapi.h marks two API numbers “WOW64 compatible”, 18 (WriteMiniDumpProcess) and 19 (QueryProcessDebugInformation), and nothing in PhSvcDispatchApiCall refuses the rest. A 64-bit client packs SERVICE_FAILURE_ACTIONS with 8-byte pointers, which a 32-bit helper would misread, so the ChangeServiceConfig2 wrap belongs to a 32-bit client talking to a 32-bit helper, or to a client built for the purpose.

The heap count

API 19, the heap query, ends in PhQueryProcessHeapInformation in phlib, which sized its output from NumberOfHeaps with the same pattern. The patch, for the newer-Windows branch:

if (WindowsVersion > WINDOWS_11)
{
    heapDebugInfo = PhAllocateZero(sizeof(PH_PROCESS_DEBUG_HEAP_INFORMATION) + ((PRTL_PROCESS_HEAPS_V2)debugBuffer->Heaps)->NumberOfHeaps * sizeof(PH_PROCESS_DEBUG_HEAP_ENTRY));
    heapDebugInfo->NumberOfHeaps = ((PRTL_PROCESS_HEAPS_V2)debugBuffer->Heaps)->NumberOfHeaps;
    numberOfHeaps = ((PRTL_PROCESS_HEAPS_V2)debugBuffer->Heaps)->NumberOfHeaps;
}

if ((SIZE_T)numberOfHeaps > (((SIZE_T)-1) - sizeof(PH_PROCESS_DEBUG_HEAP_INFORMATION)) / sizeof(PH_PROCESS_DEBUG_HEAP_ENTRY))
{
    RtlDestroyQueryDebugBuffer(debugBuffer);
    return STATUS_INTEGER_OVERFLOW;
}

heapDebugInfoLength = sizeof(PH_PROCESS_DEBUG_HEAP_INFORMATION) + (SIZE_T)numberOfHeaps * sizeof(PH_PROCESS_DEBUG_HEAP_ENTRY);
heapDebugInfo = PhAllocateZero(heapDebugInfoLength);

if (!heapDebugInfo)
{
    RtlDestroyQueryDebugBuffer(debugBuffer);
    return STATUS_NO_MEMORY;
}

heapDebugInfo->NumberOfHeaps = numberOfHeaps;

After the allocation, the function loops i up to NumberOfHeaps and writes heapDebugInfo->Heaps[i] field by field. On x86, PH_PROCESS_DEBUG_HEAP_INFORMATION is 36 bytes (it includes Heaps[1]), PH_PROCESS_DEBUG_HEAP_ENTRY is 28, and Heaps starts at offset 8. The new guard’s limit there is (0xFFFFFFFF - 36) / 28 = 153391687. One past it:

153391688 * 28  = 4294967264        fits in 32 bits
+ 36            = 4294967300        = 2^32 + 4
request         = 4 bytes
NumberOfHeaps   offset 0            inside the request
DefaultHeap     offset 4            first write past the 4 bytes requested
Heaps[0]        offset 8 onward     the loop keeps writing

So on a 32-bit build the old code was a heap overflow write sized by the loop. On x64 the sizes are 56 and 40, the guard’s limit is 461168601842738788, and a 32-bit count can never exceed it. The guard can’t fire on x64. It is there for the 32-bit build, which is the one that answers this API for WOW64 processes.

NumberOfHeaps comes from the buffer RtlQueryProcessDebugInformation fills for the target process, here a WOW64 process or a reflected clone of it. A comment in heapinfo.c says the call “injects a thread into the process causing deadlocks and other issues in rare cases”, and that System Informer queries a reflected clone to avoid them. No real process has 153 million heaps, so the wrap needs a count that disagrees with the rest of the buffer. Whether a target process can make the query report such a count, I didn’t establish.

The patch made two related changes. PhSvcApiQueryProcessHeapInformation recomputes the same length to decide how many bytes to hex-encode for the reply, and now applies the same guard before it does. It also initialises heapInfo and heapInfoHexBuffer to NULL, which the new goto CleanupExit needs, because that path can now be taken before heapInfoHexBuffer is assigned. And PhpUpdateHeapRegions in memprv.c, which runs in the UI process, got the same guard. The PR calls that one parity, which is accurate: that function stores NumberOfHeaps into its allocation and never writes Heaps[i], so a wrapped size there had nothing to overflow.

Two of the new checks can’t trigger. PhAllocate calls RtlAllocateHeap with HEAP_GENERATE_EXCEPTIONS, and its documentation says it raises an exception on failure. PhAllocateZero calls it and then memsets the result, so the new if (!heapDebugInfo) branches never see NULL outside a PH_DEBUG_HEAP build, where PhAllocate is malloc. PhBufferToHexString is declared _Ret_notnull_, so the new NULL check after it is dead in the same way. Both are harmless.

RtlULongMult

For the three ChangeServiceConfig2 arrays the patch uses RtlULongMult, from the copy of ntintsafe.h that ships in phnt:

RtlULongMult(
    _In_ ULONG ulMultiplicand,
    _In_ ULONG ulMultiplier,
    _Out_ _Deref_out_range_(==, ulMultiplicand * ulMultiplier) ULONG* pulResult)
{
    ULONGLONG ull64Result = UInt32x32To64(ulMultiplicand, ulMultiplier);

    return RtlULongLongToULong(ull64Result, pulResult);
}

The product of two 32-bit values is computed in 64 bits, where it can’t overflow. RtlULongLongToULong stores it if it is at most ULONG_MAX and returns STATUS_SUCCESS, and otherwise stores ULONG_ERROR and returns STATUS_INTEGER_OVERFLOW. It is an inline function in a header, with no system call. The failure-actions case after the patch:

if (!NT_SUCCESS(status = PhSvcpUnpackBuffer(&packedData, info, &failureActions->lpsaActions, failureActions->cActions * sizeof(SC_ACTION), __alignof(SC_ACTION), TRUE)))
    goto CleanupExit;
if (!NT_SUCCESS(status = RtlULongMult(failureActions->cActions, (ULONG)sizeof(SC_ACTION), &actionsLength)))
    goto CleanupExit;
if (!NT_SUCCESS(status = PhSvcpUnpackBuffer(&packedData, info, &failureActions->lpsaActions, actionsLength, __alignof(SC_ACTION), TRUE)))
    goto CleanupExit;
if (failureActions->cActions != 0 && !failureActions->lpsaActions)
{
    status = STATUS_INVALID_PARAMETER;
    goto CleanupExit;
}

cTriggers (triggersLength) and each cDataItems (dataItemsLength) get the same treatment. With the counts from the table, the x86 helper now returns STATUS_INTEGER_OVERFLOW before it unpacks anything. The x64 helper rejected those blobs before too, with STATUS_ACCESS_VIOLATION from the bounds check, and now fails one step earlier with a different status. RtlULongMult also puts a 4 GB ceiling on these lengths on every build, which no valid blob can reach, since the blob’s own length is a ULONG.

A count with no array

PhSvcpUnpackBuffer turns offset 0 into NULL when AllowNull is TRUE, and all three array unpacks pass TRUE. Before the patch, a blob with cActions = 5 and lpsaActions = 0 unpacked fine. The server’s own loop was skipped, since it’s guarded by if (failureActions->lpsaActions), and the struct went on to PhChangeServiceConfig2 claiming five actions with no array behind them. The client in clapi.c never sends that pair: it packs the array only when cActions != 0 && lpsaActions. The patch rejects it with STATUS_INVALID_PARAMETER right after each unpack, for all three arrays. I didn’t test what ChangeServiceConfig2 or the service control manager would have done with it. With the patch it stops at the unpack.

Reachability

What the source establishes:

• The elevated helper is started by the UI through a UAC prompt and exits 30 seconds after its last client disconnects.
• Its port’s DACL gives Administrators full access and Everyone PORT_CONNECT. The descriptor sets no integrity label.
• PhSvcHandleConnectionRequest checks the client in two build configurations. In a PH_BUILD_API build it refuses the connection unless the client process’s image file verifies as VrTrusted through PhVerifyFileEx. In a DEBUG build the client’s image path must equal the server’s. The build tool’s Release flag set includes BuildApi, which Build.cs turns into the PH_BUILD_API define.
• Once a client is connected there are no per-API checks, and the same table offers CreateService with a caller-chosen binary path, ExecuteRunAsCommand and CreateProcessIgnoreIfeoDebugger.

So a caller the helper accepts can already do what an administrator can, through the API as designed. For that caller, a parser bug in ChangeServiceConfig2 adds nothing they couldn’t get from CreateService. The privilege boundary is the connection check, and this PR doesn’t touch it. That is why the PR calls itself hardening.

What I didn’t establish: whether a process running without elevation can open the port and get through that check. I didn’t build a client and didn’t test against an official build. I’m not claiming a path from an unelevated process to any of this. The WOW64 helper runs with the token of the UI that started it, so from an unelevated UI it is unelevated too.

Upstream

PR #2879, opened 2026-04-10. On 2026-04-15 dmex merged master into the branch, approved it without comment, and squash-merged it as ca2d5b2, “Improve count-based size calculations in heap/config parsing (#2879)”. It changes SystemInformer/phsvc/svcapi.c, phlib/native.c and SystemInformer/memprv.c (85 lines added, 14 removed). The PR doesn’t mention a backport.