There is no substitute for detailed code reviews, more so if the code is related to security. Any changes to such code, however minor, should be reviewed. However, in addition, it also helps if the compiler, tools etc can help in flagging potential errors.
Take the case of the very serious security error in ios/mac software. A cursory code review would have caught this error, prevented attacks, and saved Apple from embarrassment.
Function: SSLVerifySignedServerKeyExchange()
...
if ((err = SSLHashSHA1.update(&hashCtx, &signedParams)) != 0)
goto fail;
goto fail;
if ((err = SSLHashSHA1.final(&hashCtx, &hashOut)) != 0)
goto fail;
err = sslRawVerify(ctx,
ctx->peerPubKey,
dataToSign, /* plaintext */
dataToSignLen, /* plaintext length */
signature,
signatureLen);
See if you can spot the problem.
Yes, the second "goto fail" means the SSLHashSHA1.final and sslRawVerify functions are never ever called — that is, they are unreachable.
Now, one can and should have a policy that code should be checked in only after it goes thru a company approved indentation script, and code reviews should be done only on properly indented code. This will make it easier for the author or human reviewer to spot errors — in this case, the second goto will be aligned with the following if statement. Or one could have policy that all if and else statement blocks — even one-liners — should be in braces. But all this is not foolproof and so it will be nice if the compiler refuses to compile if it finds unreachable code. Unfortunately, the C compiler is not required to throw compilation error. In all likelihood, it may give a warning but that may be lost in the other not so important warnings. You can specify compiler flags to treat warnings as error but it is typically not done as you will never be able to compile anything. But if you can get code to compile without any warnings, that would be commendable.
The Java language spec requires that unreachable code must not compile. So, this type of error cannot happen in Java — note that Java and many recent (high level) imperative, structured programming languages don’t have a goto statement as, among other things, if used unwisely, it makes it difficult to understand the program flow. The Dutch computer Scientist Dijkstra published a famous paper in CACM in 1968 titled "Go To Statement Considered Harmful"
However, one can attempt to write code in Java where some of the code is unreachable — for example, when debugging you may want to put a return statement in the middle of a method, but this will give a compilation error. However, C is a relatively lower-level, and very powerful (systems) language — the fact that it is still widely used in critical software several decades after it was created, is a testament. Every high level language’s execution speed is typically stated by how much slower it is in comparison to C. But all this power is also dangerous when not used properly — to give but one example, the number of critical patches, routinely pushed out by vendors, due to buffer overflow, is huge.
The makers of the popular Intellij IDE have not lost the opportunity to boast that their IDE can flag unreachable code
Bruce Schneier, the well regarded security expert, looks at this from a sinister angle: he asks, Was the iOS SSL Flaw Deliberate?, as it has, according to him, all the hallmarks of a backdoor — low chance of discovery, high deniability if discovered, and minimal conspiracy to implement.
I agree with Bruce on the high deniability if discovered, and minimal conspiracy to implement, but not on the low chance of discovery — the error jumps out from the page. If the version control system git was used, then git-blame command can be used to, well, find the person to blame for this!