Security Engineering Explained - Chapter 6 - Security Code Review

From Guidance Share

Jump to: navigation, search

Note - patterns & practices Security Engineering is now live at http://msdn.microsoft.com/en-us/library/ms998382.aspx.


- J.D. Meier, Alex Mackman, Blaine Wastell, Prashant Bansode, Jason Taylor, Rudolph Araujo


Contents

Summary

Security code review is an effective mechanism for uncovering security issues before testing or deployment begins. Performing code reviews helps you reduce the number of implementation errors in an application before it is deployed to a test team or to a customer. While design bugs are the most expensive to fix, implementation bugs are the most common.


This module summarizes the patterns & practices approach to code review by explaining what it is and why you should use it. It also describes the key concepts behind the approach.


Overview

A properly conducted code review can do more for the security of your code than nearly any other activity. A large numbers of bugs can be found and fixed before the code makes it into an official build or into the hands of the test team. Additionally, the code review process lends itself very well to sharing security best practices among a development team, and it produces lessons that can be learned from to prevent future bugs.


To have an effective code review, you must first understand the patterns of bad code that you want to eradicate, and then review the code with a clear idea of what you are looking for. You can use security objectives to guide the code review process.

Some vulnerability types may have elevated priority and others may be out of bounds based upon these objectives. Threat models can be used to create a more focused code review. Reviewing for specific known threats is more likely to result in bugs than a generic review that could be applied to any application.


Activity Overview

The four code review steps are shown in Figure 6.1. Each step builds upon the last, producing a result that is greater than the sum of its parts.

Image:CodeReviewSteps.gif

Figure 6.1 Code Review Steps


The four code review steps are:

  • 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 bugs and improve your understanding of where bugs are most likely to be discovered during further review.
  • Step 3. Review the code for security issues. Review the code thoroughly to find 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 that focuses on bugs 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.


Activity Summary Table

Table 6.1 summarizes the security code review activity and shows the input and output for each step.


Table 6.1: Activity Summary with Input and Output

Input

Step

Output

  • Security Requirements
  • Code (including list of changes since last review)
  • Constraints

Step 1. Identify code review objectives

  • Code review objectives
  • Code
  • Code review objectives

Step 2. Perform preliminary scan

  • Vulnerability list (false positives filtered out)
  • List of flagged areas
  • Code
  • Code review objectives
  • List of flagged areas

Step 3. Review code for security issues

  • Vulnerability list
  • Code
  • Code review objectives

Step 4. Review code for security issues unique to the application architecture

  • Vulnerability list


Techniques

An effective code reviewer makes use of the following techniques while reviewing code. These techniques are most effective when used in combination.

  • Control flow analysis. Control flow analysis is the mechanism used to step through logical conditions in the code. The process works as follows:

1. Look at a function and determine each branch condition. These can include loops, switch statements, if statements and try/catch blocks.

2. Understand the conditions under which each block will be executed.

3. 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 your application, use your code review objectives and the flagged areas from

step 2 to focus your work. The process works as follows: 1. For each input location, determine how much you trust the source of input. When in doubt you should give it no trust.

2. Trace the flow of data to each possible output, noting along the way any attempts at data validation.

3. Move to the next input and continue.


Use a Question List

Using a question-driven approach can help with the review process. Patterns & practices security guidance includes question lists for each major application type. These lists each contain a set of questions that are known to be effective during code review. Apply these questions while using control flow and dataflow analysis, and add to the questions as you learn more about reviewing code. 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.


Hotspots

Each question list is organized into a set of hotspots or trouble areas that are based on implementation mistakes that most commonly result in application vulnerabilities.


Table 6.2 shows example hotspots.


Table 6.2: Hotspots

Hotspot

What to look for in code

SQL Injection

A SQL injection attack occurs when un-trusted input can modify the logic of a SQL query in unexpected ways. As you are work through the code, ensure that any input that is used in a SQL query is validated, or make sure that the SQL queries are parameterized.

Cross Site Scripting

Cross-site scripting occurs when an attacker manages to input script code into an application so that it is echoed back and executed in the security context of the application. This allows an attacker to steal user information, including forms data and cookies. This vulnerability could be present whenever a Web application echoes unfiltered user input back to Web content.

Data Access

Look for improper storage of database connection strings and proper use of authentication to the database.

Input/Data Validation

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.

Authentication

Look for weak passwords, clear-text credentials, overlong sessions, and other common authentication problems.

Authorization

Look for failure to limit database access, inadequate separation of privileges, and other common authorization problems.

Sensitive data

Look for mismanagement of sensitive data by disclosing secrets in error messages, code, memory, files, or network.

Unsafe code

Pay particularly close attention to any code compiled with the /unsafe switch. This code will not be given all of the protection normal managed code is given. Look for potential buffer overflows, array out of bound errors, integer underflow and overflow, as well as data truncation errors.

Unmanaged code

In addition to the checks performed for unsafe code, also scan unmanaged code looking for the use of dangerous APIs, such as strcpy and strcat. Be sure to review any interop calls and the unmanaged code itself to make sure that bad assumptions are not made as execution control passes from managed to unmanaged code.

Hard-coded secrets

Look for hard-coded secrets by examining the code 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.

Web.config

Examine your configuration management settings in the Web.config file to ensure that forms authentication tickets are protected adequately, that the correct algorithms are specified on 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 the product, and they are almost never given the same level of design and test scrutiny as the rest of the product.

Threading problems

Check for race conditions and deadlocks, especially in static methods and constructors.


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.


Team Roles

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 design and intent.
  • Review the code. Review the code and find bugs.
  • Take notes. Take notes that describe the bugs 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 bugs. He or she is usually responsible for presenting the code and reviewing the code.
  • Tester. The tester reviews the code and finds bugs, and 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 bugs.


Conclusion

Security code reviews are a powerful tool to reduce the number of vulnerabilities in your application. Through the use of control flow and dataflow analysis in conjunction with a question list it is possible to find and fix implementation bugs before they are delivered to your test team or customer. Use the lessons learned in code review to update the question list and spread secure development best practices through the development team.


Additional Resources

For more information, see “patterns & practices Security Code Review Index” at http://msdn.microsoft.com/library/en-us/dnpag2/html/SecurityCodeReviewIndex.asp.

Personal tools