This is a little deviation from our usual critical systems, but considering it is a tool that heavily influences whether a guilty person goes free or an innocent one goes to jail it seems critical to me. In the State v. Chun case the defendant argued for analysis of the source code running the breathalyzer system, and they’re now available to the public. The results aren’t too surprising given the number of revisions to the system and that the software was written between 1993 and 1997. I’ll highlight a few points that I think are worth mentioning from a code quality perspective. Keep in mind how simple a breathalyzer is compared to the systems we usually talk about here.
- Inconsistent coding style: In and of itself this does not lead to code functioning incorrectly, but its a very good indicator that it will. When I’m doing a code audit and notice drastically different coding styles being used, then I know I’m probably going to have some problems to report, and they’re likely to be big ones. But even if thats not the case, and the code works perfectly, it still costs significant resources for programmers to context switch in and out of different styles, and to bring new coders up to speed on the project.
- Incorrect implementation of algorithms: This finding was a little gray in the report, one of the experts claimed that the average reading was incorrect, giving a weighted average (favoring more recent measurements) instead of a real average, while another argued that that was the intended function. Kind of sounds like arguing with a salesman doesn’t it? Either way the correct case could have been easily clarified with a simple comment in the code indicating which value was meant to be calculated.
- Automated tools: Automated tools were used as part of the auditing process. It brought back some memories seeing lint mentioned, during my undergraduate cs courses all our code had to lint cleanly (produce 0 warnings), which is very unlikely to happen in large codebases, but it is possible to minimize them, and eliminating them completely should be strived for even if its never reached. I do heavily disagree with one of the experts that classified lint and other programs like it as a development tool and not a review tool. Those two processes should feed into each other, and review is the more important one.
- Uncalled function: Uncalled functions are dangerous! From the perspective of the programmer, they’re confusing, and lots of time is wasted figuring out just what features and logic are in each build. They also can lead to errors trying to reuse code that may have been incorrectly implemented, never completed, or abandoned for other reasons. If its not used take it out of the codebase, and if its only used in certain builds create a branch in the source tree. From a security perspective leaving these things in place can significantly increase the attack surface. In a relatively recent audit, we found an entire FTP server sitting in the code that was trivial to enable. Every interaction we had with it after that was completely unexpected by the developer, and the testing clearly showed it.
Theres a few more findings, each of them with varying levels of applicability from a security/quality perspective, and the whole report is worth the read if you have time. Now I’m not personally familiar with these devices having never been on either operating end of one, but I would guess that they’re not very easy to patch, so fixing the underlying problems is nontrivial at best, and near impossible at worst for the end user… I wonder if its done over the network on newer devices? And you can bet that the fixes (estimated by one of the experts to take approximately one year to correct all the problems) are expensive for the vendor and the user.
Increasing code quality is the cornerstone of making more reliable and secure software, but seems so often to be ignored in favor of bolted on security in the form of third (or first?) party addons. And that might work or seem to, but they’ll have problems of their own, and after a while you realize that you’ve got a growing stack of sieves that still won’t hold water.