It rather involved being on the other side of this airtight hatchway: Invalid parameters from one security level crashing code at the same security level


In the category of dubious security vulnerability, I submit the following (paraphrased) report:

I have discovered that if you call the XYZ function (whose first parameter is supposed to be a pointer to a IUnknown), and instead of passing a valid COM object pointer, you pass a pointer to a random hunk of data, you can trigger an access violation in the XYZ function which is exploitable by putting specially-crafted data in that memory blob. An attacker can exploit the XYZ function for remote execution and compromise the system, provided an application uses the XYZ function and passes a pointer to untrusted data as the first parameter instead of a valid IUnknown pointer. Although we have not found an application which uses the XYZ in this way, the function neverless contains the potential for exploit, and the bug should be fixed as soon as possible.

The person included a sample program which went something like this (except more complicated):

// We can control the behavior by tweaking the value
// of the Exploit array.
unsigned char Exploit[] = "\x01\x02\x03...";

void main()
{
   XYZ((IUnknown*)Exploit);
}

Well, yeah, but you’re already on the other side of the airtight hatchway. Instead of building up a complicated blob of memory with exactly the right format, just write your bad IUnknown:

void Pwnz0r()
{
  ... whatever you want ...
}

class Exploit : public IUnknown
{
public:
  STDMETHODIMP QueryInterface(REFIID riid, void **ppv)
  { Pwnz0r(); return E_NOINTERFACE; }
  STDMETHODIMP_(ULONG) AddRef() { Pwnz0r(); return 2; }
  STDMETHODIMP_(ULONG) Release() { Pwnz0r(); return 1; }
};

void main()
{
   XYZ(&Exploit);
}

Wow, this new “exploit” is even portable to other architectures!

Actually, now that you’re on the other side of the airtight hatchway, you may as well take XYZ out of the picture since it’s just slowing you down:

void main()
{
   Pwnz0r();
}

You’re already running code. It’s not surprising that you can run code.

There’s nothing subtle going on here. There is no elevation of privilege because the rogue activity happens in user-mode code, based on rogue code provided by an executable with trusted code execution privileges, at the same security level as the original executable.

The people reporting the alleged vulnerability do say that they haven’t yet found any program that calls the XYZ function with untrusted data, but even if they did, that would be a data handling bug in the application itself: Data crossed a trust boundary without proper validation. It’s like saying “There is a security vulnerability in the DeleteFile function because it is possible for an application to pass an untrusted file name and thereby result in an attacker deleting any file of his choosing.” Even if such a vulnerability existed, the flaw is in the application for not validating its input, not in DeleteFile for, um, deleting the file it was told to delete.

The sad thing is that it took the security team five days to resolve this issue, because even though it looks like a slam dunk, the issue resolution process must be followed, just to be sure. Who knows, maybe there really is a bug in the XYZ function’s use of the first parameter that would result in elevation of privilege. All supported versions of Windows need to be examined for the slim possibility that there’s something behind this confused vulnerability report.

But there isn’t. It’s just another dubious security vulnerability report.

Exercise: Apply what you learned to this security vulnerability report. This is also paraphrased from an actual security report:

There is a serious denial-of-service vulnerability in the function XYZ. This function takes a pointer to a buffer and a length. If the function is passed malformed parameters, it may encounter an access violation when it tries to read from an invalid buffer. Any application which calls this function with bad parameters will crash. Here is a sample program that illustrates the vulnerability:

int main(int argc, char **argv)
{
 // crash inside XYZ attempting to read past end of buffer
 XYZ("x", 9999999);
 return 0;
}

Credit for discovering this vulnerability goes to ABC Security Research Company. Copyright© 20xx ABC Security Research Company. All Rights Reserved.

Comments (40)
  1. Mott555 says:

    I found a security vulnerability in the blogging software: I can type in the comment field! Someone could easily use this vulnerability to say stupid things and be offensive to other users! I demand Microsoft fix this vulnerability ASAP!

  2. Clovis says:

    Just how is an access violation exploitable? If it happens, Windows kills your process. Quite right too – in the same way as when a mouse sets off a mousetrap, the mouse needs to be terminated, not given a lump of cheese for being so clever.

    In a previous life ABC Security Research Company would've been ABC Compiler Validation Company, and the problem would've been a bug in the compiler. At least we've moved on a bit.

  3. POKE53280,0 says:

    I completely agree with this blog post.

    Moreover, I'd like to ask: why did Microsoft invent the safe CRT functions like e.g. strcpy_s? If strings are correctly validated by caller code (with respect to destination buffer sizes, too), "classic" CRT functions like strcpy are just fine.

    Thanks.

  4. Marco Schramp says:

    Here we are discussing the same thing again: blogs.msdn.com/…/555511.aspx If you break these rules, your program will fail. It's not a security issue.

  5. Pierre B. says:

    [Exercise: Apply what you learned to this security vulnerability report.]

    Easy one! What we've learned is that submitting dubious security vulnerability will cost Microsoft's security team days of work. So the vulnerability is actually a DDoS on Microsoft, easily triggerable by submitting a large volume of bogus reports from valid-sounding "genuinely concerned" customers.

  6. Billy O'Neal says:

    @POKE53280,0: The safe functions don't guard against invalid lengths. The safe functions are there to guard against you making mistakes. Because we are all human, and we all make mistakes. By explicitly tracking the size of the buffer in which a string resides (and passing to the various _s functions) you have created a scenario which will fail explicitly when you make a mistake, instead of failing silently (and corrupting memory) when you make a mistake. The _s functions have absolutely nothing to do with parameter validation.

  7. Sunil Joshi says:

    @POKE53280,0

    The problem that the Safe CRT functions were addressing was when code received input from the other side of the airtight hatch (i.e. a URL is sent to a server from the internet). Ideally, the function handling the input should have validated it to begin with (i.e. for buffer overruns and what have you) but people didn't. The safe CRT functions always null-terminate a string and require to explicitly say how big your buffers are (for memcpy_s). Hence, they build the validation in. This makes exploits like overwriting the return address by overflowing a stack buffer harder.

  8. Chris says:

    "This is also paraphrased from an actual security report"

    Let me guess, the security company who made the report is out of business already?

  9. POKE53280,0 says:

    If one validates parameters before using string functions (which quality programmers should do), the "safe" functions have no reason to exist.

  10. Sunil Joshi says:

    @POKE53280,0

    I agree. The fact is that people don't (or at least didn't) always validate correctly. It's easier in a code review (or during static analysis) to check for the safe functions than to check if people are using the standard functions incorrectly. It's also harder to get the safe functions incorrect since you have to tell them explicitly your buffer size (although admittedly people have lied about buffer sizes before see oldnewthings passim – that's more of a deliberate error that no one can save you from.)

  11. AC says:

    Another interesting thing:

    How should the XYZ function suppossedly protect itself from this "security vulnerability"?

    Sure, it can check for NULL pointers but otherwise it's impossible to check if the input pointers are bogus or not.

  12. Joseph Koss says:

    Clovis:

    An access violation in and of itself is benign, but access violations that are caused by a pointer derived from untrusted input can just as easily NOT trigger an access violation. Instead they could point to a very legal target location in the processes data segment, such as that untrusted data buffer.

  13. 640k says:

    Unlike ordinary APIs, COM API can be out-of-process, thus crashing another process. The called process can be located on a whole other server. Hello network exploit!

  14. Alex Grigoriev says:

    Exploiting access violation?

    See this: http://www.securiteam.com/…/5BP0G1FFPY.html

    Basically, a crash of a process with specifically mangled ELF module header could be used to get root privileges.

  15. ErikF says:

    @POKE53280,0:

    I like the secure CRT functions because they force me to look at the documentation when I use them (at least the first time, anyways.) Some of the traditional functions are completely broken anyways: see gets() and friends, which are irredeemably bad!

  16. Evan says:

    @POKE53280,0: "If one validates parameters before using string functions (which quality programmers should do), the "safe" functions have no reason to exist."

    If they help make the correctness more obvious — whether it be to the original programmer, other programmers maintaining the program, or static analysis tools — that is plenty of reason to exist and to use them.

  17. Depressed says:

    @640K

    If you can create a malicious COM object and persuade a target application to load it then you could, from that module, call a function with invalid paramaters and thus cause the process to crash.

    Or, perhaps, given that you've already got the code running, you could call anything from ExitProcess() to StealCreditCardNumberThenKickPuppies() and be done with it.

    "A function crashes when called with invalid paramaters" is not a damned security bug, no matter how clever you think you might be in getting the code loaded while using completely unrelated problems.

  18. James says:

    @POKE53280,0: "If one validates parameters before using string functions (which quality programmers should do), the "safe" functions have no reason to exist."

    But validating the parameters for the standard string functions isn't trivial, and it's easy to get wrong.  strncat doesn't take a buffer size; it takes the maximum number of characters to concatenate.  strncpy might not NUL-terminate.  The standard C99 version of snprintf is guaranteed to NUL-terminate, but that's not true for the Microsoft CRT _snprintf implementation.

    Moreover, by adding the "safe" versions of the functions, the compiler can easily flag the standard counterparts as being potentially unsafe to force code inspection.

    The "safe" versions are annoying when trying to write cross-platform code, but overall I think it's a good thing to make it easier for people to write code that's less susceptible to buffer overflows. (There's a proposal to make them standard in C1x anyway.)

  19. Adam Rosenfield says:

    If a program wants to deny service, it can just crash by causing an intentional access violation (e.g. *(int*)0 = 0) or call ExitProcess().

  20. MikeCaron says:

    Rule of thumb: It's not privilege escalation if no privileges are escalated.

  21. dave says:

    >Just how is an access violation exploitable? If it happens, Windows kills your process

    But it doesn't.  If an access violation happens, Windows *delivers an access violation exception* to the running thread.  The handler-of-last resort may well end up deleting the process, but there's lots of chances on the way for other code to get involved.

    A former method of choice was to have corrupted the stack in such a way that your nefarious code was entered in order to handle the exception.

  22. Cheong says:

    I feel the exercise lacks some context about the function XYZ. If that function passes values to some kernal code (such as display driver) and can make the code hangs there, it could be a real concern.

  23. Mark says:

    Cheong: it's not a concern, because you could just copy the body of the function into your own program and run it yourself. It's your process that's running these instructions; importing from a library is just a convenience.

    Alex Grigoriev: you of all people know that exploiting the core dump code is not exploiting the access violation, it's exploiting the kernel crash handler.

  24. Crescens2k says:

    For the safe string functions, I have actually seen pleanty of examples of people getting those horribly wrong too.

    The most common one is strcpy_s(dst, strlen(src), src); and doing that will negate all possible benefits this function can give. If you don't know why calling strcpy_s like that is bad then read the documentation on it again.

    Then there are pleanty of occasions where the wrong size is just passed in as the destination buffer length. Sometimes it is an easy mess up (like forgetting to multiply by sizeof(TCHAR) or something like that) but there are also lots of cases of someone thinking they are being smart by passing in a count larger than the actual buffer and then wondering why they get access violations.

    In the end, the _s functions make things easier to stop string buffer problems, but it doesn't fix everything. It is easy to say if people handle string buffers correctly then we wont need those functions, but people don't handle them correctly all the time, even with the _s functions. It can be easy to mess up, even with the extra care I put into my string handling, I've messed  up too and mostly just by a simple typo.

  25. Crescens2k says:

    -_-; Messed up my last comment a bit. You should never need to multiply buffer sizes for the _s functions by sizeos(TCHAR). I was thinking about other Windows functions which take buffer sizes in bytes at that time.

  26. Ben says:

    strcpy_s has another side-effect which is that the exception thrown is specially handled in such a way that generic "unhandled exception" handlers do not get to run. If your application performs "black box" style information gathering on a crash, it is something to be aware of.

  27. 640k says:

    Depressed:

    If you can create a malicious COM object and persuade a target application to load it then you could, from that module, call a function with invalid paramaters and thus cause the process to crash.

    Or, perhaps, given that you've already got the code running, you could call anything from ExitProcess() to StealCreditCardNumberThenKickPuppies() and be done with it.

    The other process (ntoskrnl) which is executed by "elevated user" could load the malicious object into it's process.

  28. Cheong says:

    @Mark: It's a concern because "crashing your program" and "freezing/bluescreen" your computer is different.

  29. Gabe says:

    I wonder how MS breaks the news to these earnest "security researchers" about their "vulnerability" not being real. After all, these people think they're doing a valuable service and if you insult them it could lead to bad PR ("MS ignores thousands of security vulnerabilities!").

  30. Crescens2k says:

    Gabe:

    Probably how they do it on connect for VS, Closed (Wont fix) or Closed (Not reproducable). They do that everywhere else and it doesn't cause a fuss, so Windows should be fine too.

    In all seriousness though, I would imagine that a nice detailed reason why it isn't a vulnerability would be the best way. Also thanking them for the hard work put in too. That is a constructive way of doing it because you are nicely telling them they were wrong and thanking them for doing the work at the same time.

    640K:

    COM objects don't get transferred over to the kernel side. They are always executed on the user side and individual members call code which could transfer over to the kernel side. Also, if a COM object is loaded into a process whith elevated privileges it is still not a security vulnrability, since the object will still be able to do only what it is allowed to by the user account running it. It only becomes a security vulnerability when the code becomes able to do something it shouldn't do. The problem here isn't that the object was loaded into the elevated program either, it is why that malicious object was actually created/sent in the first place. This hints that the system was already compramised in the first place.

    Cheong:

    The Windows API goes through a few user mode functions before it actually gets to the kernel, each one checking the parameters. For example, CreateFile ends up at NtCreateFile before it goes into kernel mode, so if it will crash, it will be more likely that it will be in user mode. Whats more, as I said already, COM objects don't get transferred to kernel mode. Each member call of a COM object will be in user mode so each action will follow user mode rules. So if the COM object isn't using any vulnerabilities and causes an unhandled exception, it will just happen in user mode. Passing an invalid/rouge object on it's own is just not enough to cause a real vulnerability.

    It is too heavy for the limited kernel memory stack space. The same as why C++ objects aren't used in the kernel.

  31. Crescens2k says:

    I'm thinking of giving up commenting. I've had at least one mess up in all of my past 3 comments. Anyway, I really messed the last line up. It should be:

    As to why you will never see COM objects in the kernel, it is too heavy for the limited kernel stack space. This is the same as why C++ objects aren't used in the kernel.

  32. PorkBellyFutures says:

    POKE53280,0 is correct. This is not an actual security vulnerability any more than strcpy() is a security vulnerability. But it should still be fixed.

    Every place where there is an API which, used in a certain improper way, would cause a vulnerability to appear, then you put the onus on developers not to use it that way. Sometimes it's more or less obvious (like the DeleteFile example), but it's easier to imagine developers writing code that calls an API with unfiltered input when it does not have any clear security implications, perhaps expecting an exception from the callee if there's something wrong with the data.

    Essentially, every 'bug' like this creates a new security tax that developers have to pay.

  33. Adam Rosenfield says:

    @Crescens2k: I hope you're not serious about giving up commenting because you typed the wrong thing 3 times.  You've had some very interesting comments.

  34. Mike Dimmick says:

    640k: The only way to get an interface pointer to a COM server in another process or on another server is to ask COM to get you one, either with CoCreateInstance/CoGetClassObject to create a new object, or CoGetObject to get a pointer to some existing object (or, of course, make some method call to a remote object that returns another interface pointer). When this happens, you get a pointer to a *proxy* which knows that it is representing the real object, not being the real object. When you make a method call, the proxy marshals the arguments into a buffer and sends them on to the *stub* running in the destination process. The stub unpacks (unmarshals) the arguments and calls the function, then repacks the results into a buffer and sends it back to the calling proxy.

    Obviously the proxy and stub have to understand what the arguments are and how to pack them. Integers are straightforward, pointers to buffers less so, and pointers to buffers that themselves contain pointers even less so. The creator of the interface therefore has to provide the proxy and stub code to load into the client and the server. If you only use Automation-compatible types (things you can put in a VARIANT) you can use the Automation marshaller, by providing and registering a Type Library containing the type and interface descriptions. For non-Automation types it's most straightforward to get MIDL to generate the proxy/stub code (actually it just generates tables that are interpreted by the RPC runtime).

    Proxies are also used in-process to marshal calls between different apartments or COM+ contexts, but that's unlikely to elevate privileges unless the other thread is impersonating a user with higher privileges than yours.

    If you pass some other value to a function expecting a pointer to a proxy, that function will call through the pointer just as in Raymond's example. It won't magically transport it across to the remote object, because the connection is within the proxy that you just bypassed. You can't elevate your privileges.

    Now, you could try to corrupt the proxy and see if you can invoke the remote methods out-of-order, or invoke a method on some other object altogether, but that would be a vulnerability in the remote object, not a fundamental issue with COM. If you wanted to do that you might as well just send DCE/RPC packets over the network or directly call through your own fake proxy, rather than trying to corrupt a standard proxy.

  35. Random832 says:

    PorkBellyFutures: You missed the part where there's nothing here establishing any possibility to send the invalid memory contents (and pointer to same) outside your process.

    There is no precedent for applications to reasonably interpret remote input as a pointer to an address in their own process to be used as an in-process com object. Not doing that isn't a "security tax", it's just sanity.

  36. PorkBellyFutures says:

    @Random832:

    Okay, you're right.

    It's not at all plausible that an application would ever receive data from across a trust boundary and somehow assume it to be a valid COM object. That couldn't work.

    Mea culpa, I misunderstood that bit.

  37. DodoBird says:

    I have had many a "bugs" about my function crashing when the tester passes an invalid non-zero pointer as a parameter.

  38. Brian says:

    @POKE53280,0: "If one validates parameters before using string functions (which quality programmers should do), the "safe" functions have no reason to exist."

    And if grasshoppers had machine guns, birds wouldn't eat them.

  39. Batman says:

    int main, not void main :(

    [The exploit runs before main() returns, so who cares? -Raymond]
  40. POKE53280,0 says:

    @Brian: Are you comparing *quality programmers* to grasshoppers?

    :-)

Comments are closed.