It isn’t surprising that a bug in the Microsoft Internet Explorer ActiveX controller has caused the flawed browser to be exploited since early July of this year. What is surprising, however, is the careless bug that caused it – a bad type cast, A.K.A. a single misplaced ampersand (‘&’) character.

So Microsoft owned up to the bug in their “security” blog, where they were even nice enough to disclose the offending lines of code.

The Typo

The main cause of the bug was a single misplaced ampersand, used to reference the memory address of a particular variable in C and C++ programming. But when used to denote the address of a pointer (itself a memory reference to another variable), bad things happen. Namely, security issues.

Here is the code that hackers found useful in the latest ActiveX exploit for the browser:

__int64 cbSize;
hr = pStream->Read((void*) &cbSize, sizeof(cbSize), NULL);
BYTE *pbArray;
HRESULT hr = SafeArrayAccessData(psa, reinterpret_cast(&pbArray));
hr = pStream->Read((void*)&pbArray, (ULONG)cbSize, NULL);

The first pStream->Read() call is valid – if you notice, the ampersand is used to reference the address of the cbSize variable, since Read() requires a reference rather than a pass-by-value argument.

The second pStream->Read() call, however, uses the ampersand to denote the address of `BYTE *pbArray`. oops.

pbArray is a pointer, which means that pbArray stores the memory location that contains the actual value, with the value itself referenced at ‘*pbArray’ (asterisks are used to retrieve the value a pointer points to).

So instead of passing a reference to the value stored at ‘*pbArray’, the Read() function was passed a reference to a pointer (a reference to a reference, if you will). With pbArray being an array and all, this could lead to all sorts of exploits – arrays are common causes of stack-based programming attacks and buffer overflows.

The Patch

The valid way to use the code would have been this:

__int64 cbSize;
hr = pStream->Read((void*) &cbSize, sizeof(cbSize), NULL);
BYTE *pbArray;
HRESULT hr = SafeArrayAccessData(psa, reinterpret_cast(&pbArray));
hr = pStream->Read((void*)pbArray, (ULONG)cbSize, NULL); // pbArray is already a reference

Voila, bug fixed and no more exploiting this bug. Not that there isn’t a history of exploitable bugs in ActiveX or anything

The Apology

So what did Microsoft have to say about being caught with their pants down? Well, Microsoft’s Michael Howard had this to say on his blog:

I want to drill a little deeper into casting issues. This will be a side project for me over the next few months, as I wade through bug databases and code to see if there are other related issues. I’ll also speak to various static analysis and C/C++ language experts here at Microsoft and across the industry to get their views and insight. If you have a professional opinion on casting issues, please feel free to let me know through this blog.

Translation: we’ve fucked up here and there, and I’ve got to dig through the code and find the issues.

I don’t claim to be a C/C++ god, and I understand that at 3am stuff as simple as this tend to run together. I’m just surprised that Microsoft doesn’t check behind its individual programmers a little better than that, and the fact they admitted to using an older library to accomplish this bug and even told the compiler to ignore the issue.

And all the more reason, we’re talking about ActiveX here, a security nightmare given its past history. Good job on finding the one typo Microsoft, and a crate of Red Bull awaits you as you find all the other issues in what can only be described as the worst “secure” code I’ve ever seen.