The Apple goto fail vulnerability: lessons learned (2014) | Hacker News Reader

The Apple goto fail vulnerability: lessons learned (2014)

dwheeler.com

102 pointsMrXOR39 comments

Hi, I'm the author of the referenced article. Thanks for pointing to it!

However, can you change HN thread to the article title, which is: "The Apple goto fail vulnerability: lessons learned"?

I never used the term "backdoor" in the entire article, and I certainly never claimed that this was an intentional backdoor or that it looked just like a backdoor. I said, "The Apple goto fail vulnerability was a dangerous vulnerability that should have been found by Apple." - but I never said it was intentional. I personally doubt it was intentional (it's possible, but I have no specific evidence suggesting it).

While I'm here... ask me anything (AMA)!

Flagged in hopes the title gets changed

The OP has re-used the title of an unrelated article posted a few days ago, for some reason:

https://news.ycombinator.com/item?id=25726068

Fixed now. Thanks!

(Submitted title was "The Most Backdoor-Looking Bug I’ve Ever Seen: Apple's goto fail bug (2014)". Submitters: please don't do that—it's against the site guidelines, which ask: "Please use the original title, unless it is misleading or linkbait; don't editorialize." We eventually take submission privileges away for breaking that rule, so please follow it.)

Thanks! And while I'm here, THANK YOU for all the thankless work you do. MANY people, including me, appreciate it!

Excuse me.

No problem! Thanks for pointing people to the article, I write articles with the hope that someone will read them :-).

That article is part of a series of articles called "Learning from Disaster": https://dwheeler.com/essays/learning-from-disaster.html I think we can learn from the past, and sometimes learning a story & working to gain lessons learned from it can really help.

Excuse me for changing the title of your essay. I should not do that.

The title was just my opinion. Some days ago, I read the excellent newsletter [1] of Filippo Valsorda about a Telegram's bug [2]. Yesterday, I googled for bugdoors and read about them and found this Apple's bug and your excellent essay (with many useful hyperlinks) about it.

[1] https://news.ycombinator.com/item?id=25726068

[2] https://habrahabr.ru/post/206900

> ... your excellent essay (with many useful hyperlinks) about it.

Thank you so much!

This was obviously a merge error.

I think that's very likely, but I've never seen a post mortum from Apple where they track down what happened.

I wish they would be more open about what happened. Mistakes happen. It's better for the industry if we can all learn from them.

For as long as I live, I’ll never understand style guides that permit omitting brackets around a single line following an if statement (or for, while, etc), nor code formatters that dont automatically insert them.

I agree - it should be a compile or runtime error.

Well if you compile with `-Werror -Wall` (and you should), it would throw in error in GCC 6+:

https://developers.redhat.com/blog/2016/02/26/gcc-6-wmislead...

It can make code bloated and harder to follow.

TBH it would be nice if someone made C with indent scoping instead of block scoping but I don’t think that’s truly practical with the preprocessor.

harder to follow? In what sense? Because in my book the fact that you can write both the line with braces and without make it much harder to think about, whereas in most other languages I don't need to think about it because there's only one way to write that.

What's the practical difference between:

   if (foo)
      bar;
   else
      baz;

and:

   if (foo) {
      bar;
   } else {
      baz;
   }

The latter has one extra line, which is hardly bloat. It also has the braces, of course, but the upside of this style is that it's clear where the compound statement begins (it's the line that doesn't start with a brace!), and where it continues.

They add visual noise. The grammar of C is not the same as the grammar of it's offshoots and block statements aren't part of control structures. Moreover, GCC warns about extra statements with the same indentation level with -Wall on.

IMO you get accustomed to single statement-braces very quick and they leave less room for errors.

and remove bugs

>They add visual noise.

Which is a good thing, since it prevents errors such as this one. Extra verbosity with mandatory braces is noise (to the degree that without it, the compiler could still infer what we mean), but some degree of verbosity is a good thing, as it helps make patterns in the code more evident.

In which case we might want to call it a more fitting error than the dismissing "noise".

How about "dither"?

That is probably why styles like

  if (cond) {
    // statement
  } else {
    // statement
  }

existed. Allow you to quote everything while not waste so much vertical space.

I'm so glad that modern languages are opinionated and have default formaters that come with them. Jumping from one C codebase to another is really a nightmare, especially if you want to review code. That's a security issue in my book, as you end up with more reading complexity.

I'm convinced the GNU style is just for gatekeeping at this point. You REALLY have to want to work on a codebase with that after reading anything remotely Linux inspired.

For as long at I live I'll never understand why why there wasn't a test suit to make sure that module and especially THAT module worked correctly.

Changes were made to that module without tests being run and then it was put into production code.

Meh, I worked on C at Apple and that programming style was idiomatic there. There’s nothing malicious about it, it was almost certainly a diff tool or merge error that resulted in the duplicated line.

One thing that was weird about our merges was that we always had way too many branches in flight for the current OS X, the last OS X, the next OS X beta, iOS, the next iOS, and the Windows iTunes stuff.

Even though we had a small team inside our division dedicated to releases, it's almost guaranteed there will be merge issues when managing that many forks of the same SVN or git repo.

One of the ideas to make this error visible is to forbid misleading indentation (section 3.5). It would be cool if the IDE can show this goof straight away..

These days, both GCC and Clang have a compiler warning about it:

https://developers.redhat.com/blog/2016/02/26/gcc-6-wmislead...

However, for some reason, while GCC puts it under -Wall, Clang (which Apple uses) doesn't enable it even with -Wall -Wextra; you have to manually add -Wmisleading-indentation. It's also not enabled by the default Xcode template.

Code Formating tools can help avoid this too.

At some point we should juat switch to editing abstract syntax trees instead of raw text. Then formatting is irrelevant, the structure is always clear, and each developer can choose their own way to view the code.

Well, when you edit with and require a code formatting tool as a part of your development lifecycle, you're basically there.

True, though you'd have to run the formatting tool with different rules on both checkout and commit for developers to be able to use their own preferred formats. Also keeping the syntax tree would make non-textual displays and visualizations easier.

That's basically the idea behind lisp (until you get into macros).

> Arie van Deursen argues that “code formatting is a security feature” and that indentation “white space is a security concern. The correct indentation immediately shows something fishy is going on...” [vanDeursen2014]. Arie van Deursen also argues that code formatting should be done by tools, not by hand, and shows that clearly this code was not routinely formatted automatically.

Praise then for languages with significant white space? Using a formatter tool to add the white space and a lint rule in your compiler to catch when it's not done is a bandaid for something that should be encoded into the language in my opinion. Leaving this stuff optional to enforce only eats up productivity for no decent upside.

Title of this story is heavily editorialized. The article mentions nothing about the backdoor.

>The problem with ‘goto’ is that it allows developers to create ‘flow’ in code that is unnatural... and that’s bad because developers should always strive to make code easy to read.

Those aren't mutually exclusive

The code written in my company must follow the MISRA rules. Among the others, there is a rules that requires branches to have code blocks, and another one that prohibits the use of goto.

Yes, compilers have flags, coding standards can achieve the same results. The point is that this stuff is non standard, not everybody uses GCC or Clang. Stating "this code is MISRA compliant" is stronger that "this code does not produce compile warnings with the flags x, y, z on compiler W version a.b.c".

But what tool verifies that the code is MISRA compliant? Then you’re just stating “my code is MISRA compliant according to tool W version a.b.c” ;)

I'm afraid the only working solution to prevent this type of errors is to switch to a programming language that requires you to use braces around if statement body. Static analyser is an optional tool, with output that sometimes points out to perfectly correct code, thus making it more a nuisance then a helping tool in the eyes of a time-constrained programmer.

Reply on news.ycombinator.com