Symas Blog

Recently one of our customers forwarded a code analysis report from Veracode to us. The report covered Symas OpenLDAP 2.4.12.1 installed on a RHEL4 system, and included a lot of noise about code outside the Symas OpenLDAP package. Our customer was interested in any action we would take to correct the “flaws” indicated in the report.

It seems that static code analysis tools are becoming quite popular lately; the OpenLDAP Project was included in Coverity’s scans for quite a long time. (Another tool vendor also recently offered to check our code for us, but they wished to use the results for their advertising benefit, and the OpenLDAP Foundation declined the offer.) This time since the analysis came by way of a concerned customer, I was asked to examine the report.

The report came in the form of a 57 page PDF document, in separate sections for severity of flaw, and a fairly decent description of the types of flaws in each section. It also opens with a summary count of flaws by severity. This report claimed to show 226 Very High severity flaws, 95, High, 867 Medium, and 1-2 Low/Very Low flaws. So, at first glance it looks pretty serious. (I can’t link to the actual report here, it’s Confidential….)

Of the 226 Very High flaws, 23 were reported in code shipped by Symas. The report simply lists the name of the program containing the offending code, and the source file and line number in question, along with an “Exploitability” rating which seems to be their estimation of the significance of the flaw. Which is OK in general, but for code that’s present in a common library, used by multiple programs, they list the same source line as a separate flaw in each program, when it’s still just the same piece of code. Not very helpful. Discounting duplicates, our count of 23 drops down to 11 unique locations. (At this point I can’t help comparing to Coverity’s reports, which show the actual snippets of source code in question, so you don’t have to flip around between multiple windows to see what they’re talking about. And, they automatically recognize multiple occurrences of the same source code…)

With that aside, examining each of the source lines in question, I find that their analyses are completely wrong. For example, in their very first flaw, they highlight a (correct) use of snprintf() as a potential buffer overflow, despite the fact that the snprintf function is inherently safe - that is this function’s sole reason for existence, in fact. Over the course of the next hour or so I walk through the rest of their report and find, in just about every case, that their analysis is dead wrong. (The exception is where they flag that we call unlink() without checking the return code. Correct, we do that, so their tool is not wrong this time. But in each of these cases, unlinking the pid and args file when slapd exits, or unlinking the Unix domain socket at shutdown if slapd was listening on ldapi://, the correct operation of unlink() is totally immaterial - our process is going away regardless. There’s no vulnerability here.)

I guess there’s a place in the world for static analysis tools, but tools are only worthwhile when they actually *help* you. And it seems that all too often, people rely on tools as a substitute for actual thought. In this case, I find Veracode’s tool wastes developer time drawing attention to “flaws” that are in fact perfectly valid, secure code. Now, while it’s easy for any human programmer to overlook bugs in day-to-day work, it’s also easy for any programmer to check a piece of code that was pointed out to them and see if a suggested problem actually exists. When a company like Veracode is being paid real money to essentially type “run”, walk away, and then send back the resulting output, I really question the value of whatever product/service they’re offering. If they’re sending a report like this to a customer with no programming experience, then it seems to me the real value is in having an experienced programmer review the report and provide some actual judgement on its significance. But that’s not what Veracode offers, they just turn a crank and let the results speak (in gibberish) for themselves.

(It’s also worth noting that Veracode has been analyzing a few other projects recently. I first saw their name pop up on the OpenSSL developers’ list a few weeks back, and promptly deleted the email. But for the curious, you can read it on the archive here. To me, this looks like fear-mongering to drum up business, but what do I know…)

Static code analysis can certainly be useful sometimes, but it’s really a software developer’s tool, not something useful for non-programmers. Putting this info in a customer’s hands instead of the original developers’ hands seems iffy at best. Putting bogus info out is simply wrong. (I shouldn’t single out Veracode here; ~86% of the “bugs” reported in OpenLDAP by Coverity were false as well. But at least they were responsible enough to work directly with the developers, not shop their results out to a 3rd party first.)

This is really a case where the tool can only be as good as the programmer who created it. It won’t help lazy programmers become better programmers, because it encourages lazy programmers to continue to be lazy. It won’t help poor programmers become better programmers, because its incorrect reports will foster confusion instead of enlightenment. It won’t help good programmers become better programmers, when the problems it identifies aren’t actual problems.

I’ve made a point of only using Free Software since I first started working with the FSF back in the 1980s. I’ve also made a point of contributing back for any project I found useful. (Which is why I’ve been a contributor/maintainer for GNU cc, make, binutils, etc. over the years. Any tool that helped me as a developer invariably also needed my help along the way.) In this case, since Veracode is proprietary, the only contribution I can make is public criticism. Your tool is worse than useless, because it’s wasted my time instead of helping me to work faster.

This entry was posted by hyc on Tuesday, March 31st, 2009 at 8:49 pm and is filed under Symas OpenLDAP. You can follow any responses to this entry through the RSS 2.0 feed. You can leave a response, or trackback from your own site.