Banning function calls, assurance, and retrofitting

I've been working on some C/C++ secure coding standards lately, and trying to mesh those up with the results from the static analyzer I'm using. As it turns out there is a fine line to be drawn between what you consider best practices, what a static analyzer can find, how much context the static analyzer has, and how much manual review you really want to put up with.

Let me give a specific example.

Coverity's Prevent analyzer has a number of built-in "unsafe" functions defined. The list includes the standard cast such as scanf, strcpy, strcat, etc. On top of that though they add some things that didn't make Microsoft's list; for example, rand().

I don't technically have a problem with including rand() in the list of things to be extremely careful about, but whereas it is nearly impossible to guarantee that someone has used strcpy() right, rand() actually has some pretty legitimate uses.

Consider the case where we want to do a randomized delay in processing something, or where we wish to randomly assign work to different people when it comes in, or randomly pull an item off a work queue for a customer service agent. None of these cases requires a cryptographically sound random number generator. For the most part, using rand() is a perfectly reasonable choice in this sort of situation.

When you decide that you want to ban certain function calls, or call them out in findings in a static analyzer, you're treading a fine line with your developers and the people you're going to ask to go and clean up the old code you have around. You can choose to go several routes.

• File defects against the old code for any use of a banned function, without investigating the specific use
• File defects against old code only after verifying that in the context you have a potential vulnerability
• Get a dedicated team together to just go and clean up old code

Each of these approaches has its plusses and minuses.

If you choose to file defects against your developers for code that is years old and wasn't necessarily written by them without verifying the vulnerabilities, you run the risk of them not taking the secure coding rules seriously. Many of the items they find are going to be false positives from a currently-exploitable perspective, and they are going to be cranky with you.

If you choose to go through the validate each and every defect and the types of defect are pervasive, you're going to spend almost as much verifying the defect as fixing it. Especially if you're going through and simply replacing strcpy() with strlcpy() for example. For both this and the case above though, developers are going to be going through the code, and with the proper training/awareness, they might actually learn more about some of the old code, or at least start to have a better sense of some real security issues in the code.

If you choose to get a dedicated team together to fix the old code, you're likely to save money in the short run. A dedicated team is going to get used to fixing the coding defects of this type, and you're going to make a lot shorter job of it. The downside being that the regular developers aren't getting some of the code review experience you'd really like.

To top things off, if you go route #2, wherein you only fix things that are currently exploitable, you run the risk of the code being used in a different way elsewhere and causing problems down the road.

Back to my rand() example. In every case where I've found rand() used it hasn't been in a security critical context. Do I want to leave these instances of rand() in my code as examples that others might follow, next time in a security sensitive context? Or, do I want to take a relatively dogmatic approach to this and all other banned/"unsafe" functions and eliminate them entirely from my codebase?

I'm leaning towards some sort of middle ground. If I can find the time for a dedicated team to go through the code to clean up old issues, then I'm likely to remove all instances of things that could be unsafe, just so we don't leave things around that could be copied into an truly unsafe context.

Its a tricky balancing act to determine how dogmatic you want to be about fixing up the use of certain "unsafe" functions. Getting it wrong either way can have long term consequences for your secure development program. As always, the right answer depends on a lot of factors.