The Importance of Manual Secure Code ReviewJanuary 16, 2014
In my previous post on Software Assurance (SwA), I discussed the use of automated static analysis tools to spot potential security flaws in software and noted their strengths, limitations, and costs. To get more complete coverage, we typically augment any automated analysis with a manual code review.
In this post, I'll discuss the process for performing manual secure code reviews that we have implemented and how this has resulted in more secure applications.
Manual Secure Code Review
Manual secure code review is the process of reading source code line-by-line in an attempt to identify potential vulnerabilities. It is a tedious process that requires skill, experience, persistence, and patience. Vulnerabilities discovered, and subsequently addressed through the manual review process, can greatly improve an organization’s security posture.
There are three primary phases of a manual secure code review: the interview, code review, and reporting results.
- Interview: By beginning with an interview with the developers, the review team has a chance to understand the intent of the application before reviewing the code. After first getting a basic understanding of the intended business function or capability of the application, the interview focuses on key security touch points, such as the developers’ approach to authentication, data validation, and logging.
- Code Review: After the interview, the review team works individually to review the application as a whole. Rather than handing off individual code files to specific team members, each member reviews the entire application. This approach plays to the strengths of each individual reviewer, often resulting in the identification of different, yet relevant, findings. The other advantage, of course, is that having multiple eyes on the same set of code serves as a quality check to ensure findings are valid.
- Reporting Results: After the individual code reviews are completed, the team meets to share results. Each reviewer has the chance to review the others’ findings, providing an opportunity to discuss why certain findings may appear in one team member's list but not in another's. This phase also helps to ensure that the findings being reported are relevant. The final list of findings, along with descriptions and potential mitigations, is then presented to the developers using a standard report format.
How many people should make up a code review team? At MITRE, we typically use a review team consisting of two people. One is often a senior reviewer, and the other is a junior staff member working to grow their skills in code review. A future post will discuss setting up a manual secure code review team and the dynamics of paired reviews.
Determining factors of when to perform a manual secure code review
As with automated static analysis tools, manual secure code review has its strengths, limitations, and costs.
Manual review is great at unwinding business logic and understanding the intentions of a developer. Automated tools struggle in these areas, resulting in false positives and, worse, missed issues.
Flaws related to authentication, authorization, cryptography, and overall data validation are often best identified by manual analysis.
Manual review is also good for examining rarely traversed code paths. Evaluation techniques such as penetration testing or "fuzzing-only" examine paths for which inputs are provided. Lesser exercised paths, or intentionally hidden paths, can be missed. Automated static analysis tools can follow these rare paths but often fail to understand the business logic associated with them. A rigorous manual review is typically better able to identify, unravel, and examine these paths otherwise missed or misunderstood by automated tools.
Limitations of manual secure code review
Buffer overflows (especially "off-by-one" errors), dead code, and other subtle mistakes are tough for a human reviewer to find and are better suited to automated analysis. Also, manual review can easily miss vulnerabilities that stem from the integration of many small isolated problems spread out over large code bases.
There is a cost associated with creating effective manual secure code review teams. Engineers need to be trained in the review process and must be available to perform reviews. It can take a full year of experience before an engineer is ready to effectively perform manual secure code reviews.
Because reading source code can be a slow, tedious, and fatiguing process, MITRE has formalized the process into a focused three-day review to keep the reviewer's mind fresh. Past reviews have shown that a trained reviewer can review at most 10K lines of code in three days. For applications greater than 10K lines of code, a reviewer should focus on the code related to authentication, authorization, cryptography, and overall data validation as well as other code identified as potentially troublesome during the interview with the code developers. Beyond that, other code samples should be randomly selected for manual review from the code base. This basic approach works well for code bases up to 100K lines of code. For applications larger than 100K lines of code, manual code reviews need to be heavily supplemented by automated analysis.
At MITRE, we regularly perform manual secure code reviews on custom developed applications. These reviews have identified hundreds of flaws before applications were released. By closing these security flaws before deployment, we minimized the risk of an attacker exploiting these applications.
A good software assurance program includes a variety of tools and methods. Manual review often finds flaws that no other technique can identify. When combined with penetration testing, automated analysis, design reviews, and developer education, a significant improvement can be seen in the security of an application.