goto fail;

Several Apple products have a really dumb and nasty bug that might be what let NSA do some "man in the middle" attacks. (The conspiracy minded will think it was a deliberate plant.)

Wired had some great coverage of the bug. The block in question was:

As might be apparent on visual inspection the duplicate "goto fail;" is the issue.

On my first reading of the code, though, that wasn't clear. "It's goto 'fail', what's the problem-- it'll just fail". But, of course, "fail" doesn't guarantee a "return fail;", it returns whatever the error code is. And if it's still set to zero (which is the short hand for "no error" in this context), that's what gets returned. So "fail" is more "clean up and return error code" but that's hard to type as a label. (I tend to write really long variable names and constants, so CLEANUP_AND_RETURN_ERROR_CODE is not outside the realm of possibility were I writing it.)

This code is also shown as example of the danger of not using curly braces in your conditional. Personally I'm ok with not using curly braces if and only if it fits on one line, and forms a kind of logical block. This is a bit of an idiosyncratic rule of mine, but I find
if(condition) doSomething();
fine, but
if(condition)
     doSomething();
bad, for the usual "what if you put in another line of code" etc, and situations like the goto fail.

I think the other reason I couldn't immediately spot the badness is using variable assignment and conditional test all at once. I'm more prone to use a place holder for a variable:
err = someTest();
if(err != 0) return cleanUpForFailure(err);
is how I might do that sequence, which would also get away from the ugly goto. GOTO CONSIDERED HARMFUL, as they say.

So in short, the bad practices:

• use of goto
• not using braces for conditionals (and not being on one line)
• poor naming conventions
• all in one assignment and compare

All in all, a nasty piece of work.