Threats in the coding phase¶
During coding it is easy to make mistakes. If that happens in a security control, the code may be less secure than intended. As with all other code, we can use the normal safety nets to catch bugs: (automated) unit tests, and code reviews. Quite a famous example of where a coding mistake caused a security problem is the iOS goto fail [GotoFail].
But in addition, we may choose certain programming constructs that are not secure. An example is the use of
gets() in the C programming language [Seacord]. We can catch these with the use of a secure coding standard 1.
A code security review is one of the verification methods, and could be part of the testing phase of the SDLC. However, it is so tied to writing code that we discuss it in this chapter.
Secure coding standard¶
A secure coding standard lists which programming constructs to avoid because they are insecure, and which constructs to use because they are (more) secure. It looks a bit like a threat library, but the difference is that the coding standard is very specific to a certain programming language and perhaps even programming frameworks. In the Java programming language for example, we can choose several web application frameworks and implementing security controls can be very different between them.
You may see the same threats in the coding standard as in the threat library, and the coding standard is a way to realize a mitigation for a particular programming environment. For example, the threat library may suggest to use parameterized queries to counter SQL injection, and the secure coding standard explains how to do that in PHP with Yii.
Some developers are severely allergic to the word coding standard. To them, a coding standard is a set of rules about indentation, curly braces and semicolons that are imposed by a power hungry dictator, and they feel that their time is better spent on writing good code. I can relate to this feeling, and a coding standard should not be a document for the sake of layout conformance (automated tools can do most of that for you), but a set of guidelines that developers follow in order to get higher quality code. In that sense, developer education in the rationale behind the coding standard is more important than the standard itself. Also notice that I say guidelines and not rules, if there is a good reason to deviate from the coding standard, then this should be possible. I would recommend to document such a situation somewhere in the code, especially when it comes to security relevant code. The Debian SSH bug shows how deviation from a coding standard (in this case, rules programmed into the tools Valgrind and Purify) without documenting it can lead to a quite serious vulnerability [Schneier2008].
As with the threat library, it is important to understand why we want to follow a certain guideline. Only then will we as developers follow the guidelines from the secure coding standard (or not, but then we have to give a good explanation why the code is still secure). And perhaps you still want to check the indentation, after all, it could have prevented the goto fail [GotoFail].
The secure coding standard is specific to your programming language and platform. This means that you will have compile your own. Fortunately, a lot of work has been done in this area and many secure coding guidelines exist. For example, if you use Ruby, a web search for secure coding ruby gives you a few dozen links to web pages with Ruby specific guidelines.
Code security review¶
Even with the coding standard in our hands (and heads), we can still make mistakes and choose insecure programming constructs. With the right education, a code security review (also called secure code review) is very effective ([SAFECode], p.10).
A security reviewer must understand the secure coding standard, and know what to look for. A code security review can be a harder than a normal code review, because the context in which the code is executed plays a role. Reporting a missing input validation routine on data that is trusted to begin with, is pointless.
A good way to tackle the review is to follow the control flow from the attack surface, i.e. all entry points that are available to an attacker. Another method is to search the code for possibly vulnerable constructions and see whether an attacker can exploit them. The OWASP Code Review Guide describes both approaches in more detail, including examples in several programming languages and frameworks [OWASPCRG]. Even if the authors have not updated the guide in a while, it is still useful to read.
As you can quickly get lost in the code, some bookkeeping is necessary. A good code editor, powerful search tools and something to keep notes are indispensible. If you did not write the code, make sure that someone that deeply understands the code is available for questions. I have had very productive code review sessions by reviewing code together with the developer of the code, like in pair programming.
The project’s code can change often. Reviewing the complete code with each change is not always necessary. Instead, you can perform an incremental code review, in which you review only the code that has changed. This is a lot quicker, but beware that you can overlook security issues because you did not look at the whole context of the code. Doing a more complete review once in a while is probably wise.
Communicating the code review results requires some special attention. The purpose of the review is to improve the code quality. To guarantee this in the long term, the developer needs to learn from the review. If the review communicates the message “you are doing it wrong”, this will for sure demotivate developers, especially volunteers contributing to an open source project. Respect the work of developers and realize that in the same circumstances, you could have made the same mistakes.
You can choose to report the findings in a separate report, or enter them into the project’s issue tracker. MITRE has written a sample code security review report [MITRE]. If you are only reviewing code changes (incremental review), you can use special tools that allow you to annotate the code changes.
The days that code reviewers would print out the code and mark sections with a red pen are over [UserFriendly]. Tools can make code reviews easier and faster.
A good editor can save a lot of time when reviewing code. [Sarkar] concludes that syntax highlighting significantly reduces the time that is needed to review code, so you will want good syntax highlighting.
Code navigation is also important. You will want to jump to function or method definitions, and be able to see where certain functions or methods are called from. Many integrated development environments have such code navigation, but you can also run external programs, like
ctags [Ctags] or
cscope [Cscope], or integrate them with general purpose editors such as Emacs or Vim.
During a code review, you may want to consult API documentation. Although some development environments let you browse this documentation, a normal web browser works just as well. Not all APIs document security related information and you may find yourself searching forums to find out how a certain API behaves exactly.
I have grown up using the
grep tool to search my code. Nowadays, we have tools that work better, faster and easier.
ack is an example of such a better grep, and its webpage lists an overview of other code search tools [BeyondGrep].
For larger codebases, using search engine technologies can be quicker. Tools like
ctags [Ctags] and
cscope [Cscope] are well known, but relatively new are OpenGrok [OpenGrok], Hound [Hound] and Code Search [CodeSearch].
Code review reporting tools¶
Tools to report code review findings generally work by some sort of code annotation: you add comments to one or more specific lines of code. Some tools let you only annotate code changes and are more integrated with a version control system. The tools range from plugins in your editor to web interfaces that understand several version control systems. The website Software Testing Help has made a short overview of the latter category [STH]. Probably many more systems exist or will be made in the future.
Automated code review tools¶
While code reviews are very effective, they tend to cost a lot of energy from a reviewer. Here is where static analysis tools can help. Static code analysis (SCA) tools analyze the code for specific problems. In recent years, the number of such tools has grown quite dramatically. But while this may sound like an ideal solution, there are a few caveats, so do not throw out the manual code review yet!
First of all, the static analysis tool must understand your code. If you select a tool, do not just look at language support, but also whether it supports all language features and the frameworks that you use. For example, can it understand object oriented PHP, or the application framework that you want to use?
Second, will it find security issues? There are a few ways that SCA tools search for security issues. A pattern based search is the simplest, but can be quite effective already. Taint analysis and other program analysis techniques are more computationally expensive, but will be more precise. In taint analysis, the analysis follows the data flow from a tainted source (i.e. untrusted data) to a vulnerable sink (i.e. code that an attacker can abuse). The effectiveness of such an analysis depends on what sources and sinks the SCA tool knows. For certain web frameworks this will be the case, but is this still true if you write a different kind of application?
Third, related to the previous caveat, is how accurate the analysis is. False positives are reported issues that are not an issue at all, while false negatives are unreported issues. Finding a balance between the two is a general problem with static code analysis. In general, security experts like the tool to report as many issues as possible, even if that means many false positives, while developers think the other way around. Too many false positives takes up time, and may limit the time that you expected to save. Having many reported issues is not an indication of the effectiveness of the tool, because the number of false negatives will always be unknown.
Some tools address the first two caveats by letting you tune the tool towards your environment, albeit limited. Such tuning may take time and influence the accuracy of the analysis.
Nevertheless, SCA tools can be a big help. Tools give you quick feedback and you can run them as often as you like.
Static analysis tools come in many flavours. There are open source tools and proprietary tools. Some of the proprietary tools can however be used free of charge for open source projects. There are differences in quality and the type of problems that the tool tries to find.
NIST keeps a list of static security analysis tools as part of the SAMATE project [SAMATE].
We can detect security mistakes during the coding phase with code security reviews. To catch insecure programming constructs, in addition to mistakes in mitigations, we use a secure coding standard: a programming language and framework specific list of programming constructs to use or avoid. A code security review differs from a standard code review: much more program context is needed to find mistakes. You can either start from the attack surface and follow the code or start from potentially vulnerable programming constructs and trace it back to the attack surface. Bookkeeping is essential. An incremental code review may be quick, but can miss security mistakes because you did not look at the whole context. Good tools can help the review process, ranging from text editors, search tools and static code analysis (SCA) tools. Selecting a good SCA tool can be a challenge, but using it may be worth the effort. Look at programming language support, security problems that will be found and that will not be found, and the number of false positives.
If this term makes you want to put away this guide, please bear with me, I promise it will not be as bad as you think.