See the two goto fail; statements. The second ensures subsequent statements within the same block will not be executed, making that code unreachable. The problem, as Adam points out, is that the unreachable statement below actually performs a security-critical function. By not having it called, an invalid certificate can be generated with an incorrect signature when certain types of cipher suites are used. Again, read Adam's detailed blog post on the issue.

Would Static Analysis Find This Issue?

Yes.

A successful analysis via cov-analyze, part of Coverity Quality Advisor, on the reduced code shows the UNREACHABLE checker firing. We mocked certain portions of the code so others could quickly compile and analyze the code snippet. However, the SSLVerifySignedServerKeyExchange function is structurally equivalent to the original code.

Pic or it didn't happen:

Since the code snippet is open source, it qualifies for Coverity Scan. Click here to view the project. Or if you're an existing Coverity customer, skip to the bottom of the blog for information on how to reproduce the above.

What Is the Defect?

The UNREACHABLE checker reports many instances of code blocks that cannot be executed because there is no possible control flow to reach it. These defects often occur because of missing braces, which results in unreachable code after break, continue, goto or return statements. In the affected code, the second subsequent goto fail; statement causes the control flow to always bypass the line below it, jumping to the fail: block.

Looking at the Linux Kernel, an open source project in Scan, the UNREACHABLE checker fired 112 times since the Linux kernel has been in Scan. In 2013, 33 of those defects were fixed in the kernel. And since it's been in Scan, 74 of these defects have been resolved. This is about 9 million lines of code. The UNREACHABLE checker is a good example of a high-quality checker in that even on such a large project, it's firing about once ever 80,000 lines. Noisy? Not at all.

Even still, developers may have hundreds to thousands of defects to fix in a large code base, especially one that hasn't had a static analysis tool ran on it before. So which ones should be fixed first?

How Can Developers Prioritize Such Defects?

Any defects found via static analysis in security component code should be treated as a guilty until proven innocent. These defects should take priority in triaging and remedying. This begs the question how will developers know what's a security component? Here's a great spot where developers and security engineers can work together.

Security engineers and application architects should work together to identify security sensitive components. Here's a short list:

We've blogged about quality defects occurring in security components before. MySQL also had a defect found in an authentication component, which resulted in the ability to authenticate with an incorrect password (CVE-2012-2122).

Out of all the code in an application that should be "Coverity Clean", security components come to mind. What would an UNREACHABLE do to your code base?

Showing My Math

If you're a Coverity customer, you can capture the above command with cov-build via Coverity Quality Advisor 7.0.3 since it includes clang support. You can analyze the above on older versions of Coverity Quality Advisor, such as 6.6.x. Just point to your gcc instead of the clang binary. The rest of the command should be the same.