Code Quality in Critical Systems
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.
Author: Daniel Peck
Posted: May 14th, 2009 under Big Picture.
Comments: 4
Comments
Comment from Jake Brodsky
Time: May 15, 2009, 7:24 am
When you mention writing styles, I presume you’re referring to something slightly more sophisticated than running CB (C Beautifier). Is it a comment style? Is it an issue with header files? Is it about the use of one code library instead of another?
And what do you do when the writing style is updated? Do you reformat all the older stuff you wrote?
As for automated tools, gosh I have many memories of chasing warnings. I also have discovered that many things can slip through the cracks at the link stage. Why some of the early code I wrote worked, I’ll never know. There were several data type declarations that didn’t match from one file to the next and yet everything eventually compiled.
In the end, this is what happens when people write embedded code in a white heat. There is no economic incentive to do anything different right now. So yes, the code quality probably does suck.
And how am I as a customer supposed to figure this out with embedded software, embedded firmware, and even embedded code in FPGA chips? After all, unless I can conduct an audit of the code, how can I ask for anything better?
Comment from Éireann Leverett
Time: May 15, 2009, 7:37 am
Comment from Daniel Peck
Time: May 15, 2009, 10:47 am
@Jake,
You’re right, theres a lot more to it than just running a beautifier (though thats a good thing to do just for readability). Its a big bundle of things, ranging from variable naming and declaration, to how they’re passed, 0 indexing vs 1 indexing, how each of the authors deal with pointers, the way loops are structured, etc.
As an intern during college I wrote some code for aircraft systems, non critical ones in this case, and came across the “it compiles, ship it” way of thinking much more than I was comfortable with. The way programmers and software engineers are educated has a long way to go.
As for customers, as you know auditing can be done on firmware and such, but thats not something that the customer should have to do in most cases. I do think customers need to start asking more questions and getting real answers about the development cycle of the systems they’re running. Do they have a separate QA team, are code audits preformed (either internal or third party) quartlery/yearly/not at all, does the dev team receive regular training/briefing on current vulnerabilities and how to avoid creating them, etc.
I think this can even be a differentiator in the market if its handled correctly, but they have to be demanded by the market (or congress soon, you never know with the way things are going) or they arent going to happen.
Comment from amino world
Time: May 15, 2009, 7:04 pm
this point (kinda) was made a few years ago at a PCSF conf to provide ‘business case’ justification for doing cybersecurity assessments and other “homework” for SCADA systems.
the presentation cited studied from NASA and others that due to the reasons cited above and others, there are about 6 bona-fide show stopper bugs in every 10000 lines of code — even using the best coding practices, peer reviews, and project controls available. (this may have been improved on since, as the studies had been conducted over a decade or so, as i recall… so “last century”, heh-heh.) the presentation went on to say that current PLCs and other industrial controllers had execs and kernels of about 60K-100K lines of code, so they had 6-10 bugs in their (usually never-updated) PROMS or firmware that wouldn’t be likely remedied until the systems were replaced or had major upgrades. of course, if these devices and systems are connected to a network, they are exposed to a much wider degree, thus the need to provide some layers of protection to prevent their being exploited, vertently or inadvertently.
i suspect that little has changed since this presentation was made… what, dead reader, do you think it will take to improve our situation? (besides the replacement of the existing programmer base with improved models from enlightened and/or more disciplined educational programs, that is.)
Write a comment