Blog / January 6, 2024 /

C# is a popular language in both the commercial space (think ASP.NET Core, MVC, Blazor, WPF, MAUI, etc) and the infosec space. The most well known offensive C# tools are probably those in GhostPack (Rubeus, Seatbelt, Certify, SharpUp, etc). A lot of offensive tools that target Windows use interop (P/Invoke) quite heavily to call WinAPIs for tasks such as opening handles to processes and tokens.

The default position most developers take is to use the IntPtr type in place of the native HANDLE type. The aim of this post is to show why this is not the best idea and to provide an safe alternative (no pun intended).

Let’s consider a scenario where we want to open a handle to the primary access token of a process, duplicate it into an impersonation token, and then impersonate it. If we are using IntPtr, we may define our P/Invoke functions like this (enums excluded for brevity):

public static class Win32
{
    [DllImport("kernel32", SetLastError = true)]
    public static extern IntPtr OpenProcess(
        ProcessAccessRights desiredAccess,
        bool inheritHandle,
        int processId);

    [DllImport("advapi32", SetLastError = true)]
    public static extern bool OpenProcessToken(
        IntPtr processHandle,
        TokenAccessMask desiredAccess,
        out IntPtr tokenHandle);

    [DllImport("advapi32", SetLastError = true)]
    public static extern bool DuplicateTokenEx(
        IntPtr hExistingToken,
        TokenAccessMask desiredAccess,
        IntPtr tokenAttributes,
        SecurityImpersonationLevel impersonationLevel,
        TokenType tokenType,
        out IntPtr phNewToken);
}

The biggest reason why using IntPtr is bad is because developers often forget to close these handles once they’re no longer needed, and there is nothing we can do once a reference to a pointer is lost. Consider the following example.

internal static class Program
{
    public static void Main()
    {
        var success = ImpersonateProcess(1234);

        Console.WriteLine(success
            ? "Successfully impersonated token"
            : $"Failed to impersonate token: {Marshal.GetLastWin32Error()}");
    }

    private static bool ImpersonateProcess(int pid)
    {
        var hProcess = Win32.OpenProcess(
            Win32.ProcessAccessRights.ProcessQueryInformation,
            false,
            pid);

        if (hProcess == IntPtr.Zero)
            return false;

        var success = Win32.OpenProcessToken(
            hProcess,
            Win32.TokenAccessMask.TokenDuplicate | Win32.TokenAccessMask.TokenImpersonate,
            out var hToken);

        if (!success)
            return false;

        success = Win32.DuplicateTokenEx(
            hToken,
            Win32.TokenAccessMask.TokenAllAccess,
            IntPtr.Zero,
            Win32.SecurityImpersonationLevel.SecurityImpersonation,
            Win32.TokenType.TokenImpersonation,
            out var hDupToken);

        if (!success)
            return false;

        return Win32.ImpersonateLoggedOnUser(hDupToken);
    }
}

We can see that ImpersonateProcess is neglecting to close the hProcess, hToken, and hDupToken handles whether any of the API calls are successful or not. This would typically be done using the CloseHandle or NtClose APIs. Tools such as Process Hacker will show all the handles a process currently has open.

If we did this over and over again (without closing our process), we would continue to leak more and more handles. This is a type of memory leak bug which can cause performance problems and/or crashes over time. This may also be considered a security vulnerability in some scenarios, as it introduces scope for other malicious applications to steal the handles we’ve leaked in our process’ memory.

Of course, you could just say “don’t forget”. In which case, the code could look something like this:

private static bool ImpersonateProcess(int pid)
{
    var hProcess = Win32.OpenProcess(
        Win32.ProcessAccessRights.ProcessQueryInformation,
        false,
        pid);

    if (hProcess == IntPtr.Zero)
        return false;
    
    var success = Win32.OpenProcessToken(
        hProcess,
        Win32.TokenAccessMask.TokenDuplicate | Win32.TokenAccessMask.TokenImpersonate,
        out var hToken);

    if (!success)
    {
        Win32.CloseHandle(hProcess);
        return false;
    }
    
    success = Win32.DuplicateTokenEx(
        hToken,
        Win32.TokenAccessMask.TokenAllAccess,
        IntPtr.Zero,
        Win32.SecurityImpersonationLevel.SecurityImpersonation,
        Win32.TokenType.TokenImpersonation,
        out var hDupToken);

    if (!success)
    {
        Win32.CloseHandle(hToken);
        Win32.CloseHandle(hProcess);
        return false;
    }
    
    success = Win32.ImpersonateLoggedOnUser(hDupToken);

    Win32.CloseHandle(hDupToken);
    Win32.CloseHandle(hToken);
    Win32.CloseHandle(hProcess);

    return success;
}

or this:

private static bool ImpersonateProcess(int pid)
{
    bool success;
    
    var hProcess = IntPtr.Zero;
    var hToken = IntPtr.Zero;
    var hDupToken = IntPtr.Zero;
    
    hProcess = Win32.OpenProcess(
        Win32.ProcessAccessRights.ProcessQueryInformation,
        false,
        pid);

    if (hProcess == IntPtr.Zero)
        return false;
    
    success = Win32.OpenProcessToken(
        hProcess,
        Win32.TokenAccessMask.TokenDuplicate | Win32.TokenAccessMask.TokenImpersonate,
        out hToken);

    if (!success)
        goto Cleanup;
    
    success = Win32.DuplicateTokenEx(
        hToken,
        Win32.TokenAccessMask.TokenAllAccess,
        IntPtr.Zero,
        Win32.SecurityImpersonationLevel.SecurityImpersonation,
        Win32.TokenType.TokenImpersonation,
        out hDupToken);

    if (!success)
        goto Cleanup;
    
    success = Win32.ImpersonateLoggedOnUser(hDupToken);
    
    Cleanup:
    
    if (hDupToken != IntPtr.Zero)
        Win32.CloseHandle(hDupToken);
    
    if (hToken != IntPtr.Zero)
        Win32.CloseHandle(hToken);
    
    if (hProcess != IntPtr.Zero)
        Win32.CloseHandle(hProcess);

    return success;
}

I have a few issues with both of these approaches. The first is that it doesn’t address instances where we forget to close a handle – we’re human and it’s bound to happen. The second is that the code is not very idiomatic of modern C#. The == / != comparisons are not nice ways to check handle validity; inserting multiple calls to CloseHandle increases code bloat, making it less readable; and goto is just yucky.

The better approach is to wrap the pointers in a “safe handle” class. The SafeHandle abstract class (and the higher-level SafeHandleMinusOneIsInvalid / SafeHandleZeroOrMinusOneIsInvalid classes) solve all of these sticking points as they implement IDisposable, CriticalFinalizerObject, as well as provide multiple helper methods and properties.

Such a wrapper class could look something like this:

public sealed class MySafeHandle : SafeHandleZeroOrMinusOneIsInvalid
{
    public MySafeHandle() : base(true) { }
    
    public MySafeHandle(IntPtr handle, bool ownsHandle) : base(ownsHandle)
    {
        // SetHandle is a method
        // on the SafeHandle class
        SetHandle(handle);
    }

    protected override bool ReleaseHandle()
    {
        // P/Invoke call
        return Win32.CloseHandle(handle);
    }
}

We can then use this inside the P/Invoke functions instead of IntPtr.

[DllImport("kernel32", SetLastError = true)]
public static extern MySafeHandle OpenProcess(
    ProcessAccessRights desiredAccess,
    bool inheritHandle,
    int processId);

And utilise it like so:

using var hProcess = Win32.OpenProcess(
    Win32.ProcessAccessRights.ProcessQueryInformation,
    false,
    pid);

if (hProcess.IsInvalid)
    return false;

Because SafeHandle implements IDisposable, its usage can be wrapped inside a using statement. This ensures that the Dispose method is called once the variable falls out of scope, which will then close the handle automatically. In this case, hProcess will go out of scope when the ImpersonateProcess method returns. We also have some additional quality of life features such as IsInvalid and IsClosed properties instead of == or != checks.

However, there may still be cases where we forget to utilise the using keyword or explicitly call Dispose, but this is where the SafeHandle class really has us covered because it has this destructor:

~SafeHandle() => this.Dispose(false);

But what’s so special about a destructor? Well, they are called implicitly by the garbage collector which means that the CLR will (at some point) correctly release the handle when the SafeHandle instance gets cleaned from the heap. Furthermore, SafeHandle implements the CriticalFinalizerObject abstract class.

public abstract class CriticalFinalizerObject
{
    protected CriticalFinalizerObject() { }

    ~CriticalFinalizerObject() { }
}

It has no direct functionality, but what it does do is tell the CLR to guarantee that all finalization code in a derived class is given the opportunity to execute, even in situations where an exception is thrown or the hosting AppDomain is forcefully unloaded.

You even don’t have to create your own wrapper class, as C# already has some built in ones, including SafeProcessHandle, SafeFileHandle, SafePipeHandle, and SafeRegistryHandle. Using these instead of IntPtr will make your code much safer.

c#