Static analysis tip: How to resolve statically detected defects

Matthew Hayward, Coverity

January 21, 2009

Matthew Hayward, CoverityJanuary 21, 2009

Because statically detected defects rarely have an external advocate (like a customer) demanding that they be fixed, the management of a development organization needs to make the adoption of a static tool a priority in order to reap the benefits.

Even in organizations where management has taken the bull by the horns and made time for static analysis, they can be thwarted by the unique challenges of resolving statically detected defects.

Let me offer an illustration. One static analysis tool, Coverity Prevent, enforces the following rule: "The BAD_COMPARE checker finds many cases where a function is implicitly converted to its address and then compared with 0. Because function addresses are never 0, such comparisons always have a fixed result."

Here is a line of code that was flagged by this checker in a commonly used open source application:

if (getuid() == 0 || geteuid != 0)

Please take a moment to determine the severity of this defect on a scale from "minor" to "worst case scenario," in terms of security. Now, of course this is something of a trick question: no one could possibly answer that question without access to the surrounding code context and an understanding of the design intent of the code in question.

However, this particular defect caused a very serious security vulnerability in the package in question (long since fixed). While Coverity Prevent gets the credit for detecting the defect, the credit for correctly identifying the severity and resolving it goes to an open source developer.

An appropriate defect resolution process for a statically detected defect is:

1. Consider the design intent of the code in question
2. Consider the behavior of the code in question in the case identified by static analysis
3. Change the code to make it match the design intent

This is substantially different from the defect resolution approach for a customer reported defect, which would be something like:

1. Determine what piece of code is causing the user reported misbehavior
2. Change the code to stop misbehaving

The primary difference is in the impossibility of correctly assessing the severity of statically reported defects, or to proposing a resolution, without understanding the design intent of the code in question. This means knowledgeable, well-trained developers are much more likely to succeed at resolving static analysis issues.

Changing code to match the design intent is critical and making changes that silence the static tool may actually be harmful to product quality. Most static analysis tools will flag an issue in the code below; but let's consider how a developer, presented with this issue by static analysis, might proceed.

1 int chop(char * in) {
2 int idx;
3 if (in) {
4 idx = strlen(in);
5 if (idx) idx--;
6 }
7 // Bug! What if in is
8 // NULL!
9 in[idx]='\0';
10 return idx;

There are a few reasonable changes -- either moving code within the conditional, or adding some error handling code, but no matter what, the only way to fix the defect is to determine what should happen when the in variable is null, and alter the source code to behave in that manner. Only a developer familiar with the application can make such decisions correctly.

Matthew Hayward is Director of Professional Services for Coverity Inc. Since he joined the company in 2005, he's worked with hundreds of development organizations to define and implement strategies for effectively applying static analysis to improve software quality and security. He holds a M.S. in Computer Science, and B.S. degrees in Mathematics and Physics from the University of Illinois.

Loading comments...