Understanding the Aesthetics of Apple’s SSL Bug

Apple’s latest SSL bug (which impacts both iOS and Mac OS) is covered in detail by the venerable Adam Langley on his blog. My blog post is much less ambitious and focuses on one question: if we evaluate the bug aesthetically, without any knowledge of the circumstances that led to it, does it appear more like a bug or more like a backdoor?

The indented if statement

The if statement which contains the two goto fail statements is indented in a way which makes it easy to immediately assume that both goto fails are being evaluated as part of the statement itself. However, the if statement is not bracketed! This is interesting because sslKeyExchange.c (the file where the vulnerable code occurs) uses a bunch of if statements, and many of them are indeed bracketed. For example:

if (message.length < 2) {
    	sslErrorLog("...");
        return errSSLProtocol;
}

This bracketing happens out of necessity, since we are executing more than a single line of code as part of the if statement. However, since our vulnerable if statement is not bracketed (it doesn’t start with a { and end with a }), only its first instruction gets executed as part of the if condition. The second goto fail line is executed whether the if statement’s conditions are satisfied or not.

It’s interesting to note that had the programmer used an initial if statement in that method followed by else if statements, their code would not only most likely be more efficient when compiled, but the second goto fail statement would have been detected as invalid by the compiler. This vulnerability would have then been avoided. But the programmer went for a less efficient method which made the bug possible.

The repeated goto statement

Generally, when there is a security bug in a piece of code, it is the case that the programmer wrote the bug falsely thinking that they were contributing something positive to the code (this is what happens every time I am the source of a security vulnerability).

However, there is no reasonable practical reason that I can imagine for putting two goto statements, that point to the same method, right after each other. I simply can’t come up (and I don’t think anyone can) with a use case where this could be useful. This is because once the first goto is identified, execution jumps to (or goes to, so to speak) the referenced method and continues from there, end of story. The second goto statement is never needed and can’t possibly be construed as offering something useful, even innocently. So why was it added in this diff (line 631)? What was the reasoning? This is an unanswered question.

In conclusion, the especially significant circumstance here is that we had an unjustifiable repetition of a goto instruction which can’t reasonably be falsely construed as useful, inside an indentation that made it appear as part of an if statement that already included that instruction in the first place.

Posted February 22, 2014 by Nadim in Computing, Security