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 (yet again)


It's the bogus vulnerability that keeps on giving. This time a security researcher found a horrible security flaw in Sys­Alloc­String­Len:

The Sys­Alloc­String­Len function is vulnerable to a denial-of-service attack. [Long description of reverse-engineering deleted.]

The Sys­Alloc­String­Len does not check the length parameter properly. If the provided length is larger than the actual length of the buffer, it may encounter an access violation when reading beyond the end of the buffer. Proof of concept:

SysAllocStringLen(L"Example", 0xFFFFFF);

Credit for this vulnerability should be given to XYZ Security Labs. Copyright © XYZ Security Labs. All rights reserved.

As with other issues of this type, there is no elevation. The attack code and the code that crashes are on the same side of the airtight hatchway. If your goal was to make the process crash, then instead of passing invalid parameters to the Sys­Alloc­String­Len function, you can launch the denial of service attack much more easily:

int __cdecl main(int, char**)
{
    ExitProcess(0);
}

Congratulations, you just launched a denial-of-service attack against yourself.

In order to trigger an access violation in the Sys­Alloc­String­Len function, you must already have had enough privilege to run code, which means that you already have enough privilege to terminate the application without needing the Sys­Alloc­String­Len function.

Once again, we have a case of MS07-052: Code execution results in code execution

Earlier in the series:

Bonus bogus vulnerability report:

The Draw­Text function is vulnerability to a denial-of-service attack because it does not validate that the lpchText parameter is a valid pointer. If you pass NULL as the second parameter, the function crashes. We have found many functions in the system which are vulnerable to the same issue.

¹ Now, of course, if there were some way you could externally induce a program into passing invalid parameters to the Sys­Alloc­String­Length function, then you'd be onto something. But even then, the vulnerability would be in the program that is passing the invalid parameters, not in the Sys­Alloc­String­Length function itself.

Comments (23)
  1. Alex says:

    I would like to report a serious vulnerability in Windows. memcpy_s does not check the dest buffer is actually of the size numberOfElements! It is trivially easy to create a buffer overflow using code similar to the following:

    memcpy_s( pLittleUnassumingBuffer, 0xFFFFFFFF, pHaxorBuffer, HaxorBufferSize );

    This issue must be resolved with the utmost haste!

  2. Spike_tt says:

    I've noticed from reading StackOverflow that when using the C++ language it is trivially easy to invoke "Undefined behaviour".  In this state apparently your code can do anything.  The example often quoted is that it can format your hard drive.

    What are Microsoft doing about this vulnerability in Windows?

  3. farseerfc says:

    Isn't this the same issue as the exercise part of the episode 1?

    [They are all the same issue as the exercise part of episode 1! -Raymond]
  4. Tim says:

    It'd be nice if you tagged the secure hatchway posts so I'd have a link to send the occasional overzealous security person that I run into.

  5. Mason Wheeler says:

    Bonus bogus vulnerability report.

    * Closed with status code ID-10T-058: User is unaware of the "Fail Fast Philosophy" principle of software design.

  6. MacIn173 says:

    The last one bit reminds me about one issue I've seen once, maybe someone can comment it. When you work with ini files using functions like WritePrivateProfileString etc, some functions accept optional parameters. In case parameter is optional (omitted) NULL is passed instead. But it turns out that if you pass a valid pointer to a buffer that has 0 length asciiz string (first byte is 0), it crashes by trying to write(!) to that buffer. To me it seemed absolutely valid to pass valid pointer to an empty string, but seems that it is not. So I had to add a wrapper so that if strlen == 0, NULL would be passed instead of actual pointer. Any ideas if this is valid behaviour?

  7. Joshua says:

    Well assuming it really is WritePrivateProfileString, that's kind of a bad bug, writing to LPCTSTR.

  8. Mark says:

    Oh man, I hope Nicholas Lemonias never discovers Windows. This sounds just like the stream of nonsense "vulnerabilities" he's been posting to FD.

  9. Tim! says:

    What's with the "denial-of-service attack" wording?  I am only familiar with this in the context of interconnected machines on a network.

  10. Darran Rowe says:

    Well, it is what you'd expect from the network. A say you have a local web server on your system, again the denial of service is not being able to use a web browser to load pages from that web server.

    Really, a server is just a program that provides a service of some kind, like web pages or streaming music or even a corporate program to work with data. A client is a program that connects to the server to use the services it provides. The network context has very little to do with denial of service.

  11. 12BitSlab says:

    @ Tim!

    Usually when we think of DOS or DDOS, we assume it is an attack against a network.  However, rouge/broken processes on a machine can effectively deny operation of that machine and other processes on the machine when CPU, memory, disk, or other critical items are completely consumed.  

  12. Evan says:

    "However, rouge/broken processes on a machine can effectively deny operation of that machine and other processes on the machine when CPU, memory, disk, or other critical items are completely consumed."

    Or on a crash. For example, consider a system that runs a print spooler daemon in the background. The process continually lives, accepting print requests and forwarding them to printers. (I don't know if this is how spoolers work in general.) If I can send a malformed document to the spooler and cause it to crash -- thereby denying the ability to print to other users of the system -- that's still a denial of service attack.

    The "overwhelm a server with a ton of requests" is only one specific kind of DOS attack.

  13. Roman says:

    > [Long description of reverse-engineering deleted.]

    That's my favourite part. Apparently they needed to reverse-engineer the function to come up with that "vulnerability".

  14. Myria says:

    Sometimes I question the "It rather involved being on the other side of this airtight hatchway" philosophy for the rest of Microsoft: parts of Microsoft seem to consider code signing to be a security boundary, and some parts don't.

  15. Sven2 says:

    The timeGetTime function has a security vulnerability where dereferencing its return value can cause an access violation depending on system clock state. For example:

    #include <Windows.h>

    #include <UIAutomationCoreApi.h>

    UiaCacheRequest r = *(UiaCacheRequest*)timeGetTime();

  16. Scarlet Manuka says:

    Raymond, I know you've often said in the past that even vulnerability reports that seem to be obvious nonsense, like this one, must be investigated in depth, in case there is a real exploit being masked by poor reporting. Can you share with us (suitably anonymised if necessary) any case where that investigation actually turned up a real exploit?

    I have been wondering about the cost-benefit ratio of all that extra investigation, so it would be very interesting to see an example of a case where it did make a difference, and turned up something that otherwise might not have been found.

    [It was something like, "It sounds like they are saying that the problem is X (which is bogus), but maybe they really mean Y (even though they never mentioned Y)" and it turns out that Y was a genuine issue. Another case was "Their report and proof of concept demonstrates X (which is bogus), but a deeper investigation shows that you can trigger X from inside the airtight hatchway via sneaky technique Y (not mentioned in the report), which is a real problem." What I don't know is whether the finder really meant Y and was bad at communication, or whether they really meant X and just got lucky that Y was a few steps away from X (even though they didn't know what those steps was). It's like calling in a false alarm to the fire department, and it turns out there was a fire three houses down the street. -Raymond]
  17. cheong00 says:

    Btw, is there a few missing F in the value?

    Since the length parameter us uint, I would think 0xFFFFFF is a safe value to pass in.

    Afterall, this value is sufficiently smaller than the actual limit of uint. (The parameter use unsigned value suggested that it's safe to use values even if with leftmost bit set). And the function documentation said the function will return null if no memory blocks that can fit the size is found. I would have possibly go just check for null after read the documentation.

    Not a security vulnerability, but possibly a bug in the function if that's not a typo.

  18. boogaloo says:

    "Since the length parameter us uint, I would think 0xFFFFFF is a safe value to pass in."

    @cheong00: Yes 0xffffff is perfectly safe to pass in, as long as you pass in a string that is 0xffffff characters long. The example passes the text "Example" that is 7 characters long, so you ideally shouldn't pass in a higher value than 7.

    msdn.microsoft.com/.../ms221639(v=vs.85).aspx

    "Allocates a new string, copies the specified number of characters from the passed string, and appends a null-terminating character."

    The string length can't always be checked as there is no requirement that it is null terminated. So you could plaster the function with a load of time wasting validation that might catch some failures and promote them to just returning null, which the application could then add another load of time wasting validation to catch. The result will be a greater chance of bugs and when it does work it will be slower, I'm sure marketing would love to put that on the box. The people who wrote the bug report have no business writing software (idioms.thefreedictionary.com/have+no+business+doing dictionary.cambridge.org/.../have-no-business-doing-sth).

  19. 12BitSlab: It doesn't even to have be rogue program: I've had numerous deadlocking issues I've had to resolve; that's effectively a program DoSing itself.

  20. Mr Cranky says:

    I hope "XYZ Security Labs" got the public credit they demanded.  It's good to know who's the best at detecting bogus vulnerabilities.

  21. cheong00 says:

    @boogaloo: Oh, I misread the main point.

    I read the documentation and found it will append a null character to resulting string, and then assume it's about "when allocating a buffer the size of UINT_MAX, it'll crash instead of just returning null".

  22. boogaloo says:

    There is a vulnerability in Microsoft Visual Studio as it allows you to write, compile and link malicious code. The same vulnerability is also found in GCC and clang.

    Proposed fix(*). Prevent linker from generating files that can be executed.

    (*) this is a joke, my actual proposed fix would be to demote, redeploy or sack a developer who doesn't understand undecidable problems.

    en.wikipedia.org/.../Undecidable_problem

Comments are closed.

Skip to main content