Tiny typo blamed for massive IE security fail

One small typo in Microsoft's code caused the security vulnerability that prompted Microsoft to release an out of sequence patch on Tuesday, it has emerged. A rogue ampersand ("&") created a security hole in a the MSVidCtl ActiveX control that hackers began exploiting early this month. A blog posting on Microsoft's Security …

[Anonymous Coward]

Cue anti C/C++ rants...

C/C++ is insecure , shouldn't be used blah blah buffer overflows pointers are a Bag Thing (tm) blah blah ignorant waffle blah blah everyone should use managed C#/Java/Python/VB [delete as applicable to your IQ in descending order]

There, said it, you can all go back to your high level languages in the safe knowledge that your code , compilers and runtime p-code interpreters are all bug free.

[Destroy All Monsters]

@boltar

No You!

"The extra ‘&’ character in the vulnerable code causes the code to write potentially untrusted data, of size cbSize, to the address of the pointer to the array, pbArray, rather than write the data into the array, and the pointer is on the stack. This is a stack-based buffer overrun vulnerability."

Yeah. It's like, why do I have to parse this kind of bullshit when I am not reading about FFT implementations for signal processors? Clearly there is something wrong with the way wrapped assembler code is used in 2010.

[Anonymous Coward]

@ Cue anti C/C++ rants...

[quote]Bag Thing ???[/quote]

Unfortunately it seems even a genius such as yourself is susceptible to ‘mistakes’, so maybe it is better to have another fail safe. Who knows

[quote] [delete as applicable to your IQ in descending order][/quote]

Charming.

[Ken Hagan]

Does not surprise me

Had Microsoft used a weakly typed language, the run-time for that language would have saved them. Had they used a strongly typed OS interface, the compiler would have saved them. But this is Windows, so we have zillions of casts converting all pointers to LPVOID and all integers to DWORD. As you reap...

[Sean Timarco Baggaley]

@boltar

The problem with C / C++ is that they're not high-level languages and were never designed for the kinds of uses they're being put to today.

Using these languages to write high-level stuff such as ActiveX controls is wrong. End of story. Use the right damned tool for the job.

Of course, we're using operating systems and legacy code written at a time when C and C++ were still considered The One True Way. This leads to a lot of readily available libraries and lots of documentation. Sadly, this means there's tons of mediocre code for mediocre programmers to just copy and paste into their own source files. Which explains so much about the quality—or lack of it—in today's software.

(And yes, I do know a number of assembly languages, C, C++ and Objective C. I used to write games on 8-bit and 16-bit microcomputers back in the day; I'm not ranting from a position of ignorance.)

The older generations of the C family do have their place, but writing application code for complex modern operating systems isn't one of them.

[Tim Bergel]

This is a stack-based buffer overrun vulnerability

No it isn't, its a stupid coding error that is writing the data (whether or not it overruns) to the wrong place.

[Anonymous Coward]

Errr

This may be a dumb question, but shouldn't any half decent compiler in any half decent language be able to tell where a reference to a variable is wanted, as distinct from a reference to the *address* of that variable?

As I understand it, the ampersand (&) changes it from one to t'other, and with a decent compiler and decent coding standards and decent programmers, having the ampersand where it doesn't belong (or missing one out) would lead to a compilation warning which would get fixed before it became a security hole so bad that it deserves an "out of sequence" Windows Update.

Oh, I forgot. It's software. Normal product liability rules [1] allegedly don't apply. In fact, as it's Microsoft, normal legal jurisdictions seem not to apply.

[1] Quick intro to UK product liability (EU and US aren't that different, unless you're a lawyer):

http://www.businesslink.gov.uk/bdotg/action/layer?topicId=1074002228

[Crazy Operations Guy]

Must be...

Solar Flares, causing a magnetic instability on the hard disk causing a particular set of bits in the code.

[Anonymous Coward]

Typo?

This strikes me as more of an 'i don't really know how it works' error. A rogue & is not a typo, it's someone cocking up their pointers through carelessness, or not understanding them.

[frank ly]

Job Title?

"...originally pinpointed by reverse engineer Thomas Dullien..."

Should that be 'reverse engineering expert'; or is there something strange about Thomas Dullien?

[Michael Fremlins]

I have a question

If this bug is "difficult to spot" with access to the source code, how did hackers spot it and exploit it?

What tools or methods are they using to find these vulnerabilities?

[Anonymous Coward]

Code errors revisited

Rather reminiscent of the classic mainframe operating system (IBM MVT about Release 18) blunder where a new IBM programmer thought he would save 2 bytes of code in the 4-byte program IEFBR14, whose assembler code read (with comments)

SR 15,15 clear return-code register to zero

BR 14 branch to the address in register 14

The programmer removed the first instruction, and this update was included in a (much bigger!) update sent to all IBM customers.

The result was that the initiator/terminator invocation (for that is what the program did) now returned a random value rather than the expected zero value, and consequently everyone's jobs fell apart.

History does not record the fate of the programmer - and obviously such a thing could never happen now...

[Rod MacLean]

Typical

No-one seems at all surprised to find MS have schoolboy coding errors in their software.

What does that tell you?

[FoolD]

Untrusted Data?

"write potentially untrusted data"

Does it really matter if the data was trusted or not - it was being written to the wrong bleedin place!

Very worrying that a) Microsoft's programmers can't tell the difference between an array variable and the contents of an array and b) such a bug got past any type of testing - the code must surely of never worked. One can only assume it was in a conditional branch that was rarely hit.

Proof that the new 'secure' CRT functions will never work - so long as the pointer to a buffer and the buffer length are not intrinsically tied together it will always be possible to make a simple typo and pass garbage the functions.

This could be achieved by use of simple classes to control access to memory most of the time (a built-in Buffer class to replace malloc would be a good start) but I can't see people like Microsoft going back and re-writing all of their legacy code to use them.

A better solution might be to move memory management into hardware - in such a way that raw pointers know, or can get to, the length of the data they point to. I can't see software ever being 100% safe until this is done. One could then just raise an exception if data is written out of bounds in hardware, 'fixing' 99% of current code, including CRT functions.

/one can dream

[Anonymous Coward]

"move memory management into hardware"

Indeed, sensible thoughts, but unlikely to happen.

VMS's native languages have a faciity called "descriptors" where the programmer references the descriptor of an item rather than the item itself, and the compilers and/or run time libraries (which are trusted) do the actual manipulation. As the descriptor contains the item type, its address, and the available space, buffer-overrun-class errors are difficult, and safe type conversions are easy. There's nothing VMS-specific about the concept.

ps

the code must surely of never worked

would better off been written

the code must surely have never worked.

[Filippo]

how did it work in the first place?

No, really? This isn't a matter of C++ vs C#. This is code that has never been tested once. Adding an unnecessary & will almost always cause the function to crash spectacularly the first time it's used.

If the code wasn't even tested *that* much, chances are that using a strongly-typed, managed, whatever language, would have only meant putting a bandaid on the real problem - namely, that no compiler in the world can save you if you won't test your code.

[E 2]

What lang would you write an OS in?

Seriously.

Java = slow but runs anywhere there's a Java runtime. Oh, wait.

Python = runs anywhere there's a Python runtime. Oh, wait.

Assembler = you think C++ is too much for normal people?

Smalltalk & spawn = you are mad.

How many Python & Java runtimes/interpretters, and how many assemblers, are written in their native lang? Most of theme are written in C or C++!

C++ is a fine programming language. So is C. The reason they are still around is that they are fine languages. Programmers make mistakes while coding in all languages, it's not the fault of the language.

Entirely aside: IMNSHO, if you can't handle pointers and addresses and reference variables, then there was something lacking in your education. Memory addresses are fundamental to computers.

[K. Adams]

Shadows of the AT&T Telecom Crash of January, 1990

These types of errors are all too easy to make...

Even something that should be "easy" to spot, like the improper nesting of "IF - THEN - ELSE" or "SWITCH - CASE" statements (to use pseudocode vernacular - I know C statements should be lowercase), can slip past trained eyes, only to come back and bite you in the ass sometime down the line.

For those who are old enough to remember (and who lived in the US at the time), an erroneously placed "BREAK" statement in a nested block of "IF - THEN - ELSE" C logic caused most of the AT&T's 4ESS telephone switching network to crash in January, 1990.

The "IF - THEN - ELSE" block was nested inside of a "SWITCH - CASE" block, with the "BREAK" statement residing inside the "ELSE" sub-block, when it should have resided **outside** of the "IF - THEN - ELSE" block entirely. The coding error had the effect of starting a race condition in a telephone switch accepting offloaded calls from another, heavily-loaded switch. This caused the backup switch to offload its calls to yet another switch, propagating the error through the network.

The Public Switched Telephone Network (PSTN) was brought to its knees in a matter of seconds, and wasn't fixed until AT&T reverted all of its 4ESS switches to an earlier version of code, some ten hours later.

So if it's easy for a key part of decision logic to slip past a trained programmer's eyes, it's understandable how something as small as a rogue ampersand can go undetected.

Mine's the one with the C Language Reference in the pocket...

[Anonymous Coward]

*(&fubar)

i remember at the first C shop i worked at, new hires who didn't understand pointers would simply add or remove asterisks and/or ampersands until they got their code to compile. if it subsequently crashed in testing they would just rinse and repeat.

you'll be pleased to hear that many of them are still in the industry!

[Mike 16]

About those tests...

Last time I tried to use MSVC (admittedly almost a decade ago), you had two choices:

1) Stifle most warnings

2) Avoid including Windows .h files

I suspect MSFT uses option (1)

'nuff said?

[Dustin 1]

Was this an outsourcing error?

Can we find out? I supect it is. One thing I have noticed in the past at a large company was that Dev was always roughshod and QA was mearly functionality testing. There was rarely any code review. Me thinks MS should hire more qualified well paid developers and spend more time reviewing code. More eyes the better. I mean every line of code should be reviewed by a peer. Expensive yes, just dont pay the execs so much and you can afford it.

[Michael Fremlins]

@E 2

An OS written in Java? JX. Or JNode.

You do have a point, but also it depends what you mean by "OS". I wouldn't class ActiveX as an OS, and MSVidCtl is just a control. ActiveX is also meant to be language independent, which is exactly why controls can be written in C++, VBasic, Delphi and others.

This looks like a really simple typo, the sort anyone can make. It does surprise me a little that it wasn't spotted. Do MS not have any code review? Or did Bob the programmer do it all himself?

[Anonymous Coward]

Re: Cue anti C/C++ rants...

C++ a language under control of a committee - and it shows. MFC - someth that shows how bad programming can be made

[Anonymous Coward]

Gotta love C++

One of the reasons I hate working with it... A tiny typo and everything just goes mental!

I'll stick with a nice strictly typed like Delphi thank you very much!

@K. Adams - Ah, the infamous overload cascade... I never did hear the exact reason... Love it!

[Richard 12]

@E 2 - this isn't part of the OS

It's an ActiveX component used to play video - just like the Flash or Real Media plugins.

The two scary things about this have already been mentioned:

1) The code cannot have ever worked as it must have always failed to pass the data correctly, therefore the path was never tested.

2) As it's a specific path, it's an edge case and therefore should have had an obvious 'whitebox' test.

It's also very silly to be doing this kind of code in simple C/C++. While there's nothing wrong with the language, the compiler doesn't protect you against silly typos.

In a strongly-typed language, the compiler will catch you attempting incorrect dereferencing by typo, while C/C++ will just let you do it and happily copy the wrong stuff to the wrong place.

However, you could use 'good' classes that do real checks on what's arriving (do I expect a pointer or a pointer to a pointer?).

Personally, I really like Delphi - it's how object-oriented should be, as you usually simply pass objects around. I know you're really passing pointers all the time but the compiler forces you to specifically say what you think you're passing. On top of that, it gives you the ability to check at runtime what a function really received, and react accordingly.

On the downside, it's Pascal - Begin and End gets old fast. Although that is the only downside I can think off off-hand.

[Kanhef]

How it might work

Since * and & are on adjacent keys, this could have been a simple typo: "*&ptr = data". A very smart compiler might throw a warning, but it will compile and execute. Another possibility is if there's a function func(foo *a), and someone wrote "func(&data)", thinking that data is of type foo, not foo *. That should generate a warning over incompatible pointer types (foo * vs foo **), but might have been compiled anyway.

Normally, this sort of thing should never make it through testing. Overwriting an array pointer will usually crash quickly or corrupt enough memory that it will be noticed later. However, since this pointer is on the stack, it is relatively short-lived and the function may return before anything bad happens. Still, rigorous testing should have discovered that data is not being written into the array properly.

[Pete 8]

OMG PPL WTF

Programmers are boring aye.

Carry on ranting...

[Dr Patrick J R Harkin]

I've made that mistake when coding.

Can I now claim to be working to the MS standards?

[jwernerny]

Automated Code Inspection

I wonder if the automated static code analysis tools (QAC, Coverity, etc.) would have caught this mistake. I know the one we use tends to catch a fair number of "stupid programmer tricks." (tm)

Of course, nothing finds bugs like shipping the first unit. :-)

[Stephen Byrne]

A typo? how cute.

"I contend that this would be very difficult to spot in a code review, and is not picked up by the C/C++ compiler owing to the (void*) cast. If the cast is removed, the compiler issues a warning like this:

C2664: '<function>' : cannot convert parameter 1 from 'BYTE **' to 'BYTE *'

I despise C-style casting because it’s utterly unsafe; C++ casting is safer, although the reinterpret_cast operator is almost as bad as C-style casting"

Once you cast to a void *, all bets are off. Blaming it on a "typo" is a nice, cute,

way of sweeping the real problem - that they are STILL, after all this time, essentially coding in C - under the carpet.

[call me scruffy]

So, Language Zealotry asside,

Had anyone done anything outlandish like.

* Not blow the casts to hell

* Aim for a warning free "clean compile"

* Use case test

* Whitebox test

* Regression test

This bug would never have escaped into deployment.

It pains me when I see something that clearly wasn't tested before release, I've even seen crap get into firmware and mask roms.

[BioTube]

Copy/Paste fail

Can happen in any language; the issue here is that C was designed when simple bounds checking was horribly expensive and as a result pushed a lot onto the programmer whose time was cheap compared to the runtime cost. C's original design as a system language helped in this error, since that role requires the ability to remove type barriers. Of course, C++'s requirement to cast to and from a void*(which I never could understand) is what made the error undetectable: a C compiler would've happily converted, but sparked an incompatible pointer warning.

[Anonymous Coward]

Firmware

@scruffy:

Bad code in firmware is the rule, not the exception. Mobile phones, PC BIOSes, GPS units, vending machines, industrial robots... you very rarely find good programmers in companies that don't view themselves as software companies.

[call me scruffy]

Re:Firmware

Yeah, I know.

Bloody mentality of only our core business matters. Of course the canteen food being safe is a legal requirement, the duty first aiders being trained is a legal requirement, but Software, HR, ohhhh boy.

[Anonymous Coward]

@Anonymous Coward 14:06 GMT

If we're getting picky on grammar:

'would better off been written'

would have been better written

'would have been better written'