How To: Perform a Security Code Inspection for Managed Code (Baseline Technique)
- J.D. Meier, Alex Mackman, Blaine Wastell, Prashant Bansode, Jason Taylor, Rudolph Araujo
- Managed Code (.NET Framework 1.1)
- Managed Code (.NET Framework 2.0)
This How To shows you how to perform security code reviews. This module presents the steps involved in the activity, and techniques for analyzing your results. Use this How To with Security Question List: Managed Code (.NET Framework 2.0) and Security Question List: ASP.NET 2.0. These companion question lists help you ask the right questions when performing a security code review.
This How To module will help you to:
- Learn how to effectively analyze code for security issues.
- Identify the type of issues that have security implications for your code.
- Generate a list of security issues found in the code that should be prioritized for repair.
A properly conducted code review can do more for the security of your code than nearly any other activity. A code review allows you to find and fix a large number of security issues before the code is tested or shipped. In addition, the code review process allows the development team to share security best practices and experience, which can prevent future security issues.
Before you conduct your code review, you should understand the patterns of bad code that you want to eliminate. You can then review your code with a clear idea of what you are looking for. If the application you are reviewing has stated security objectives, make sure that you are familiar with them. Some vulnerability types can have elevated priority and others can be out of scope based on your security objectives.
Rather than waiting until the end of a project and reviewing everything at one time, review your code each time there is a meaningful change. This allows you to focus on what has changed rather than trying to find all of the security issues at once.
If you find that you are spending too much time on any one piece of code or functional area, especially if it is not a high priority, flag it for later review and move on.
How To Use This Module
Use this module to conduct an effective code review for security. When you use this module, keep the following in mind:
- Set clear objectives for your review. A focused review is an effective review. Spend time at the beginning of your review to understand the security issues that are possible in the code that you are reviewing.
- Set a time limit for your review. During your code review, you might find that the details become overwhelming, and you may lose track of the higher-level security vulnerabilities that you are looking for. Set a reasonable time limit on your review, and then optimize your review for this limit. If you find yourself spending too much time in any one area (especially if it is not a high-priority area or objective), flag it for later review and move on.
- Use the question list. Use the companion question lists, [.NET Framework 2.0 Security Inspection Questions]] and ASP.NET 2.0 Security Inspection Questions to help ask the right questions to identify security issues in Step 3, "Review Code for Security Issues."
- Review incrementally and iteratively. Limit your reviews to small, manageable pieces of code. This allows you to finish quickly, stay focused, and find a larger number of security issues in the code you are examining.
- Review only for security. The more objectives you have for a review, the less likely you are to meet any of them. Stay focused on the discrete set of security objectives you have for the code review. Focus on performance, reliability, and functionality issues during separate reviews.
- Know your application architecture. A good understanding of the application's architecture can make your code review more effective. At the least, make sure that you understand the component architecture of the application before you begin. Understanding the dataflow from the user, between components, and to any data repositories can also be very helpful during the review.
- Update your coding standards. During successive code reviews, identify key characteristics that appear repeatedly and add these to your development department's coding standards. Over time, this helps raise developer awareness.
At a minimum, the code must be available when you conduct the code review. Additionally, the following can help:
- Architecture or component diagrams
- Data flows
- Data schemas
The output of the code review activity is a set of identified vulnerabilities ready to be prioritized for repair.
This How To presents a four-step code review activity, as shown in Figure 1.
Figure 1. Security code review steps
The security code review activity includes the following steps:
- Step 1. Identify security code review objectives. Establish goals and constraints for the review.
- Step 2. Perform a preliminary scan. Use static analysis to find an initial set of security issues and to improve your understanding of where you will be most likely to find security issues when you review the code more fully.
- Step 3. Review the code for security issues. Review the code thoroughly with the goal of finding security vulnerabilities that are common to many applications. You can use the results of step 2 to focus your analysis.
- Step 4. Review for security issues unique to the architecture. Complete a final analysis by looking for security issues that relate to the unique architecture of your application. This step is most important if you have implemented a custom security mechanism or any feature designed specifically to mitigate a known security threat.
Step 1. Identify Security Code Review Objectives
The purpose of this step is to set goals and constraints for your code review. While you should not spend too much time setting objectives, do not begin reviewing a large body of code without knowing what you are looking for. While it is possible to conduct a code review without setting goals, this increases the chances of getting overwhelmed by the code and decreases the chances of finding security problems.
When you set objectives, you should know both the types of security issues that are common for the application you are reviewing as well as any specific code changes that should be reviewed. For example, when you review a source file for the first time, you may be interested in a subset of the following categories (depending on the functionality of the code under review):
- SQL injection
- Cross-site scripting
- Input/data validation
- Sensitive data
- Code access security
- Exception management
- Data access
- Unsafe and unmanaged code use
- Undocumented public interfaces
During a later review, you will most likely be interested in an even smaller subset of these categories based on the changes made since the last review.
For an effective security code review, set goals and constraints for the following:
- Time. Set a time limit. This helps you to avoid spending too much time on a single issue. Performing multiple, smaller reviews is more effective than one long review.
- Types of issues. Determine what types of issues you are looking for. For example, consider the following:
- General issues that affect confidentially, integrity, and availability.
- Issues related to your application's security quality of service requirements.
- Issues related to your application's compliance requirements.
- Issues related to the technologies that your application uses.
- Issues related to the functionality your application exposes.
- Issues you found from threat modeling if you have performed this activity. For steps on threat modeling, see http://msdn.com/ThreatModeling.
- Out of scope items'. Identify what you will not be looking for. Explain why these things are out of scope. For example, it is not important to look for SQL injection issues if your application has no interactions with a database.
Determining Code Review Objectives
To determine the objectives for your review, consider the following questions:
- Which common coding errors apply to the code you are reviewing? Create a list of the technologies used in your application. Pay special attention to the architecture to see what other components your application interacts with. Is there a database? Does your component present data on a Web page? Does your component interact with native code? Do users supply input to your component, either directly or through an intermediary? Use this list to help you focus on the set of issues you are interested in searching for.
- 'What is the scope of your review? It is important that you know what code you will be reviewing and what you will not' be reviewing.
- If you have a threat model, which of the identified threats apply to the code you are reviewing? After you determine which threats apply, you can separate the threats into two categories: those for which the risk has been mitigated and those for which the risk has not been mitigated. Make a list of those threats for which the risk has been mitigated. Any countermeasure code you might have written should be included in your security code review.
Example Security Objectives
The following are examples of security code review objectives:
- Make sure that all untrusted input to the component is passed to a validation routine before it is used.
- Check error handling to make sure that exceptions are caught consistently and caught close to their source.
- Check calculations whose results are used for memory allocation or buffer access for numeric overflow or underflow.
- Check cryptographic routines to make sure secrets are cleared quickly.
It is better to conduct multiple short reviews on small pieces of code, for example at the time of check-in. If you have a large backlog of code to review, it is even more important to set a time limit on the review. Code reviewing is detailed, tiring work, and it is easy to start making mistakes after many hours of review. Also, without a time limit, you may become engrossed too deeply in the details of a particular implementation. By setting a time limit, you can force yourself to move on to find high-value issues elsewhere. Another useful trick is to perform a code review with a partner. The resulting discussion and extra set of eyes can keep you focused for much longer than you can manage on your own.
Focused code reviews are effective code reviews. You should look at the code with specific goals, time limits, and knowledge of the issues you want to uncover. Not only will this substantially increase your chances for success, it will also reduce the amount of time you spend reviewing.
Step 2. Perform Preliminary Scan
In this step, you perform a scan of the code to find an initial set of security issues and to discover hot spots where additional security issues are likely to be discovered in later steps. You may need to perform the following two types of scans:
- Automatic scan
- Manual scan
You can combine these scans or you can perform just one of them, depending on your time limits and review objectives.
The purpose of an automatic scan is to find security issues that could be missed during a manual review. However, an automatic scan can result in a large number of false positives, and will not find every security issue that you might find in a manual review.
You can use a static analysis tool to perform an automatic scan of your code. Use the static analyzer to find a first set of security issues and to improve your understanding of where issues are likely to be discovered manually. Theoretically, anything a static analysis tool finds can be found by manual review as well. However, static analysis tools are unique in that they test the code without knowing or requiring any external states to be set. Because a static analyzer tool does not know what the application or function is intended to do, it will not make assumptions that a developer or code reviewer might make. Static analyzers tend to be good at finding careless code practices, such as missing error handlers, empty catch blocks, integer overflows, and scoping problems.
If you do not have access to a static analysis tool, you can perform text searches on your code base: for example, by using the Findstr command-line tool.
Note Security issues tend to cluster. If your scan finds a large number of security issues in a particular component or function, then you should carefully examine that area to discover security issues that may have been missed on the first pass.
- Managed Code Considerations
Managed code takes care of many of the security issues that scanners have been good at finding. In native code, you could scan reasonably accurately for buffer overruns, format string problems, use of potentially dangerous Win32 APIs, memory leaks, and so on. While managed code eliminates some types of security issues, there are still numerous problems that can occur, such as scoping problems, integer overflows, lack of cloning, exception handling, data truncation, lack of null checks, and unchecked values used for memory allocation or buffer access.
- Limitations of an Automatic Scan
Do not expect an automatic scan to do more than locate surface errors. While automatic scanning can supplement a manual review; it cannot replace it. Even the best scanners have contextual problems. They are good at finding security issues that are caused by single lines of code, reasonable at finding security issues that span multiple lines of code in a single function, and generally bad at finding security issues whose scope spans multiple functions.
- False Positives and False Negatives'
Due to its programmatically rigorous nature, a static analysis scan may find problems that a manual review will miss. However, these analysis tools frequently find false positives. While these can be frustrating, you can gain a better understanding of the code you are reviewing by reviewing the automated scan results. The review forces you to understand why a false positive is false, which can give you a deeper understanding of the code, including control and dataflow. On the other hand, be careful not to develop a false sense of security if an automated scan shows no security issues in your code. This does not mean that your code is free of security vulnerabilities.
You should complete a manual scan of your code to better understand the code and to recognize patterns that will assist you in Step 3. This should be a quick scan that takes no more than 10 percent of your total code review time. In particular, you should review with the following questions in mind:
- Input data validation'. Does the application have an input validation architecture? Is validation performed on the client, on the server, or both? Is there a centralized validation mechanism, or are validation routines spread through the code base?
- Code that authenticates and authorizes users. Does the application authenticate users? What roles are allowed and how do they interact? Is there custom authentication or authorization code?
- Error handling code. Is there a consistent error handling architecture? Does the application catch and throw structured exceptions? Are there areas of the code with especially dense or sparse error handling?
- Complex code. Are there areas of the code that appear especially complex?
- Cryptography. Does the application use cryptography?
- Interop. Does the application use interop to call into native code?
The result of this scan is a set of areas that deserve further analysis in Step 3.
Step 3. Review Code for Security Issues
In this step, you manually review the code to find security vulnerabilities. You should look for common security vulnerabilities that are not unique to your application's architecture. You do this by tracing those paths through the code that are most likely to reveal security issues. You can use a question-driven approach in conjunction with techniques such as control flow and dataflow analysis.
Note These techniques are most effective when used in combination.
Use a Question List
Using a question-driven approach can help with the review activity. This How To includes a set of questions that address the most common coding vulnerabilities and are effective to use during code review. Ask these questions while you are using control flow and dataflow analysis. Keep in mind that some vulnerabilities require contextual knowledge of control and data flow, while others are context-free and can be found with simple pattern matching.
Refer to the following:
Combine the following techniques when you review the code.
- Control flow analysis. Control flow analysis is the mechanism used to step through logical conditions in the code. The process is as follows:
- Examine a function and determine each branch condition. These can include loops, switch statements, if statements, and try/catch blocks.
- Understand the conditions under which each block will execute.
- Move to the next function and repeat.
- Dataflow analysis. Dataflow analysis is the mechanism used to trace data from the points of input to the points of output. Because there can be many data flows in an application, use your code review objectives and the flagged areas from Step 2 to focus your work. The process is as follows:
- For each input location, determine how much you trust the source of input. When in doubt, you should give it no trust.
- Trace the flow of data to each possible output. Note any attempts at data validation.
- Move to the next input and continue.
Input and Output
While performing dataflow analysis, review the list of inputs and outputs, and then match this to the code that you need to review. Some common sources and sinks are:
- Public interfaces
- User interface
- Database interaction
- Socket interaction
- File I/O
Note Prioritize any areas where the code crosses trust boundaries; for example, where the code changes trust levels.
It can be difficult to determine how much you trust each input source. Your code should not trust input that comes from outside your component, and your code should fully validate all data. For performance and maintainability reasons, however, this may not always be practical. In general, you can trust code that you are most familiar with or that comes from within your enterprise, and give less trust to code that you are less familiar with. The following is an example of how to think about trust boundaries.
- Place high trust in the following:
- Input from code you are reviewing inside the component.
- Input that comes from known good, strongly named, managed assemblies or signed/hashed native libraries.
- Input from a database that is used only by your component and that contains data which you can prove has been properly validated.
- Network data that has been signed by a known good source and is protected by IPSec or SSL.
- Place medium trust in the following:
- Input from known good assemblies or native libraries that have not been strongly named or signed, but are local to your server.
- Input from a public interface that should only be accessible to trusted users.
- Input from a user interface component that should only be accessible to trusted users.
- Network data that should not be accessible to an untrusted user, such as a segmented LAN internal to your datacenter.
- Place low trust in the following:
- Input that comes from assemblies or native libraries that have not been strongly named or signed and are located on the client.
- Input that comes from client code.
- Input that comes over the network.
- Input that comes from a file.
- Input that comes from a public interface that is accessible to any user.
- Input that comes from user interface component that is accessible to any user.
- Input that comes from a database that is shared with other applications.
As you conduct your traces, carefully examine the code to make sure that input validation is performed rigorously on low-trust input and performed adequately on medium-trust input. You should have a set of common validation routines that your application can call as soon as it receives any untrusted data. This gives your application a central validation area that can be updated as new information is discovered.
When you perform dataflow analysis, pay attention to areas where the data is parsed and may go to multiple output locations. Also pay attention to intermediary output locations. For example, input can go to a database and then later be placed in Web page content. Trace data back to its source, and assign trust based on the weakest link.
The question list provided in the companion modules [.NET Framework 2.0 Security Inspection Questions]] and ASP.NET 2.0 Security Inspection Questions are organized into a set of key areas, or hotspots, that are based on the implementation mistakes that result in the most common application vulnerabilities. These hotspots are summarized in Table 1.
What to Look For in Code
A SQL injection attack occurs when untrusted input can modify the semantics of a SQL query in unexpected ways. As you review the code, make sure that the SQL queries are parameterized and that any input used in a SQL query is validated.
Cross-site scripting occurs when an attacker manages to inject script code into an application so that script code is echoed back and executed in the security context of the application. This can allow an attacker to steal user information, including forms data and cookies. This vulnerability can be present whenever a Web application echoes unfiltered user input back to Web content.
Look for improper storage of database connection strings and proper use of authentication to the database.
Look for client-side validation that is not backed by server-side validation, poor validation techniques, and reliance on file names or other insecure mechanisms to make security decisions.
Look for weak passwords, clear-text credentials, overly long sessions, and other common authentication problems.
Look for failure to limit database access, inadequate separation of privileges, and other common authorization problems.
Look for mismanagement of sensitive data by disclosing secrets in error messages, code, memory, files, or the network.
Pay particularly close attention to any code compiled with the /unsafe switch. This code does not have all of the protection that normal managed code has. Look for potential buffer overflows, array out of bound errors, integer underflow and overflow, as well as data truncation errors.
In addition to the checks performed for unsafe code, also scan unmanaged code for the use of potentially dangerous APIs such as strcpy and strcat. For a list of potentially dangerous APIs, see the section "Potentially Dangerous Unmanaged APIs" in [.NET Framework 2.0 Security Inspection Questions]]. Be sure to review any interop calls as well as the unmanaged code itself to make sure that bad assumptions are not made as execution control passes from managed to unmanaged code.
Look for hard-coded secrets in code by looking for variable names such as "key", "password", "pwd", "secret", "hash", and "salt".
Poor error handling
Look for functions with missing error handlers or empty catch blocks.
Examine your configuration management settings in the Web.config file to make sure that forms authentication tickets are protected adequately, that the correct algorithms are specified in the machineKey element, and so on.
Code access security
Search for the use of asserts, link demands, and allowPartiallyTrustedCallersAttribute (APTCA).
Code that uses cryptography
Check for failure to clear secrets as well as improper use of the cryptography APIs themselves.
Undocumented public interfaces
Most undocumented interfaces should not be in your application', and they are almost never given the same level of design and test scrutiny as the rest of the code.
Check for race conditions and deadlocks, especially in static methods and constructors.
Step 4. Review for Security Issues Unique to the Architecture
In this step, you look at the list of code review objectives, and examine anything that has not yet been reviewed. This step is especially important if your application uses a custom security mechanism or has features to mitigate known security threats. Use this final code review pass to verify the security features that are unique to your application architecture. A question-driven approach produces the best results. Consider the following questions:
- Does your architecture include a custom security implementation?
A custom security implementation is a great place to look for security issues for these reasons:
- It has already been recognized that a security problem exists, which is why the custom security code was written in the first place.
- Unlike other areas of the product, a functional issue is very likely to result in a security vulnerability.
- Are there known threats that have been specifically mitigated?
Code that mitigates known threats needs to be carefully reviewed for problems that could be used to circumvent the mitigation.
- Are there unique roles in the application?
The use of roles assumes that there are some users with lower privileges than others. Make sure that there are no problems in the code that could allow one role to assume the privileges of another.
When you review code that supports multiple user roles, you must understand what each role should be allowed to do. Prepare a roles matrix that specifies privileges in rows and roles in columns. Mark each cell that corresponds to a privilege allowed by a role. (See Table 2 for an example.)
After you have completed the matrix, review the code for exceptions to this matrix. Even a well-designed system with clearly drawn roles can be broken by a bad assumption or a logical mistake in the roles implementation.
Role Matrix Example
What to Do Next
After you have completed your code review, do the following:
- Prioritize the security issues found. Prioritization should be based on the impact the security issue could have on your customers. Think through the maximum damage potential, and determine which of your customers will be affected.
- Evaluate the risk of fixing the security issues. Each security issue fixed can introduce a new, unknown security issue. Sometimes the security issue you know is less dangerous than the security issue you don't.
- Learn from your mistakes. Keep a running dialog with your team about the mistakes made, how they were found, and how they were fixed. Strive to write code that is security issue free.
Security Question Lists
Use the following questions to help you perform code reviews.
- .NET Framework 2.0 Security Inspection Questions. This question lists organizes a set of questions to help identify potential security issues when building applications with managed code (.NET Framework 2.0).
- ASP.NET 2.0 Security Inspection Questions. This question list organizes a set of questions to help identify potential security issues when building applications with ASP.NET 2.0.
Code Review Scenarios
There are several strategies for conducting code reviews, including:
- Individual Review. This strategy assumes that a single person will review the code.
- Team Review. This strategy assumes that multiple people will review the same code. This can be a highly effective code review strategy, but it requires additional organization to be successful.
In either strategy, you can select a reviewer who is familiar or unfamiliar with the code.
The advantage of using a reviewer who does not have prior knowledge of the code is that he or she will examine the code with fresh eyes and will be less likely to make assumptions than someone who is more familiar might be.
The advantage of using a reviewer with knowledge of the code is that he or she will be able to find subtle errors that require expert familiarity with the application under review.
During a code review, there are a several distinct tasks:
- Present the objectives. Describe the application/component purpose and security objectives.
- Present the code. Walk through the code and describe its use and intent.
- Review the code. Review the code and find security issues.
- Take notes. Take notes that describe the security issues found, any open questions, and areas for future investigation.
Typically, the following roles are involved:
- Business analyst. The business analyst describes the application purpose and security objectives. He or she is usually responsible for the presenting the objectives from a business standpoint.
- Architect. The architect describes the application purpose and component architecture. He or she is usually responsible for presenting the objectives from a systems architecture standpoint.
- Developer. The developer describes code details, reviews the code, and finds security issues. He or she is usually responsible for presenting the code and reviewing the code.
- Tester. The tester reviews the code and finds security issues. The tester typically has less code knowledge but a more destructive, break-the-code mind-set than the developer has. He or she is usually responsible for reviewing code and recording issues.
The Microsoft Engineering Excellence Group (EEG) is tasked with improving the methodology used to engineer software at Microsoft. The group is responsible for delivering a common baseline of prescriptive and authoritative guidance with focus on people, process, and tools. After reviewing lessons learned from across the company for successful code review, EEG promotes the following across Microsoft:
- Focus on defects. Look for defects in the artifact (in this case, the code), and resist the urge to think about and talk about solutions.
- Use a moderator. Arrange for someone to facilitate the review process and meetings. This leads to more efficient reviews.
- Prepare individually. The majority of defects are found when each reviewer spends time looking for defects on their own. Meet only to collect the data.
- Use checklists. Personal and team checklists serve as a reminder to review the artifact for the most frequent mistakes. Personal checklists have proven to be particularly effective because people are creatures of habit and tend to repeat the same mistakes.
- Use conservative rates. If you rush through a review, you may lose the value of the review. You might not get a solid understanding of the material, causing you to miss key issues, and you might tend to focus on the wrong issues, such as coding style. You might also show that you are not interested in reviews by doing a cursory job. In practice, EEG has found that a higher volume of significant defects is found when reviewers limit themselves to 250–500 lines of code per hour.
- Continuously improve. Do perform root-cause analysis for each defect, and adjust your processes, tools, or skills to avoid recurrence of the problem in the future (for example, update checklists). Dedicate time in each review meeting to discuss possible improvements.
- Measure effectiveness. Use metrics to judge the effectiveness of a review, such as personal and team yield. Alternatively, judge reviews by the following rule: if most reviewers found the same defects, the review was effective. If most reviewers found different defects (that is, there are not many in common), there may be more defects waiting to be found.
- Provide support materials. Reviewers must have a complete understanding of the artifact. Provide API documentation, design documentation, and other supporting materials to aid reviewers in getting a full picture