Abstract

Security code reviews allow development teams to leverage the biggest advantage they have over attackers – in-depth knowledge of the design, architecture and source code for the application.

Introduction

In the first part of this article, we talked about the concepts and methodology around threat modeling. As explained in that article, threat modeling is a structured approach for identifying, evaluating and mitigating risks to system security. It aims to view the system as an attacker would but with the added advantage of knowing the internals of the application under test.

This article builds on that conceptual foundation and presents the second piece in a software security code review i.e. code inspection. As the name suggests code inspection involves reviewing the application’s source code while looking for common software security problems in a systematic manner. Most real-world applications however typically consist of hundreds of thousands if not millions of lines of code and reviewing every line of source in such a large code base can be a enormous time consuming, costly and hence impractical effort. This is where threat modeling again can help.

With a list of prioritized assets and threats, from the threat modeling exercise, the development team must work to map these back to actual source code artifacts. Then based on the amount of resources available – both in terms of man-hours and cost – the team can determine which are the most critical or high risk areas of the application and focus their efforts on those specific areas of the code. The rest of this article will describe the process of code inspection, what teams should look for and how to use the findings from such an analysis.

Security Code Reviews

Before, we dig into the details of code inspection, it is important to consider the team responsible for performing this inspection. In our experience, the best candidates to be members of this team are developers themselves who understand the architecture of the application at hand. A number of our clients use peer reviews between developers to achieve this. We usually recommend that the reviewers have significant development experience while also being trained and educated in secure coding practices. They need to be aware and thoroughly abreast of the security policies and standards that developers need to adhere to[1][1]. It is not uncommon to find members of the corporate security departments responsible for code inspections especially while developers come up to speed with secure development practices. In our experience, however at least in the long run, it is important to have someone in the development team responsible for this function. As a compromise we often find a team comprised of representatives from both major stakeholders.

Tools

Like most other security functions, security code inspection can be automated to some extent. A number of free and commercial tools are available to help in this regards. For instance, CodeAssure from Secure Software (http://www.securesoftware.com) helps to perform automated code inspection for a number of common security vulnerabilities. Among the free scanners, one of the most popular scanners is RATS[2][2]. These tools can primarily be classified into static or dynamic analysis tools. The static tools essentially scan the code looking for unsafe functions and other language constructs. These typically are far more effective with unmanaged languages such as C and C++ where the list of unsafe functions is well documented. Dynamic analysis tools makers claim that their tools compile the source, and determine call graphs and data flow patterns to provide a lower false positive and negative rate than their static analysis counterparts. In our experience however, most tools have limited effectiveness. While they are useful in finding low hanging fruit quickly, they rarely are effective in finding complex authorization flaws for instance. Further, they are most lacking in dealing with managed languages such as Java and C# which take issues such as buffer overflows out of the spectrum for the most part. Also the classification systems that these tools use often does not take into account exploitability. For instance, in a recent code review engagement, a commercial tool we used to scan the code base reported a number of “critical” findings such as classes being not marked as sealed and keys being left in memory. However, manual code review revealed far more significant and exploitable issues including total system compromise by anonymous users. At Foundstone we have been able to build a host of internal scripts to automate the scanning for unsafe functions. These are relatively easy to create and typically can be made most effective through the use of regular expressions and scripting languages such as Perl.

At Foundstone another tool that helps us significantly with the code inspection process is .NETMon which is a part of the larger Foundstone S3i .NET Security Toolkit[3][3]. This tool is a code profiler for .NET application including those that use ASP.NET. The tool allows for setting custom filters to control what information is profiled. For instance, one can configure .NETMon to only profile methods belonging to the System.Console class. During a security code inspection, this tool is an excellent way to monitor calls being made to security significant APIs including those that are part of the .NET common language runtime (CLR). For instance, we can check what calls are being made to the System.Security namespace, whether access control checks are being made before any access is allowed to a protected resource. This is useful in finding blind spots and backdoors where authorization checks maybe bypassed.

Findings

As part of the security code inspection, we attempt to identify four classes of issues:

Security Flaws

A security flaw is an issue that has come about due to an inappropriate design decision or implementation choice. They are generally architectural or design problems and have global implications. An example of a flaw would be a user authentication system that does not encrypt passwords in the data store. Moreover this class of problems often stem from the fact that the design specification was itself incomplete or didn’t address the issue specifically. Thus, it is quite possible that a flaw might exist in an application even though the developers have implemented the design specification completely and correctly.

Security Bugs

A security bug is a code level problem. Often semantic in nature, they are usually the result of a coding error or bad implementation of a design decision. An example of a bug is a buffer overflow where the memory allocated is not sufficient to hold the data being passed to it, resulting in data potentially being written to privileged memory locations. These issues typically tend to be language specific.

Commendations

A commendation is a note of good security. Our experience is that this is as important to note the positive security attributes as it is the negative. Positive enforcement encourages repetition of good practices.

Recommendations

There are many ways of designing software. These findings list some considerations for future revisions that would facilitate enhanced or improved security. We also use this category of issues to convey any insight we have into better software development practices that the application developers can adopt. For instance, specific software or architecture design patterns might be recommended.

The remainder of this article will now discuss how to perform effective and efficient code inspections and how to use the findings discovered as a result of the review.

When tasked with performing a code review on a large application the first action item is to attempt to compile the code base. The advantage of doing this is that you can debug the application if necessary to narrow down or verify a potential bug or flaw. Often times this is not possible though since we may not have access to critical libraries or custom third party components or development environments. The other advantage of compiling the source code is that it lets you leverage the extensive capabilities of the development environment. For instance, if using Microsoft Visual Studio.NET, intellisense can act as a tremendous aid during the code inspection process. Eclipse and other development environments have similar features.

Foundstone Security Frame

When looking at the security significant areas of the code base it helps to approach it in a systematic manner to avoid getting overwhelmed. At Foundstone, we have built a simple frame of reference and checklists that enable a person auditing the code to go about his / her task in the best possible manner. Each section of code being reviewed for vulnerabilities and threats that belong to the following widely accepted vulnerability categories:

·         Configuration Management: As part of this category, consider all issues that stem from insecure configurations and deployment. For instance, it is important to review the web.config file for all ASP.NET applications to check any authentication and / or authorization rules embedded there. Another common configuration flaw to lookout for is how the framework and application deal with error messages i.e. whether detailed error messages are propagated back to the client. Similarly, it is important to also ensure that by default debug information and debugging are disabled. Other examples of such configuration settings include the validateRequest and EnableViewStateMac directives in ASP.NET. Configuration management checks also include verifying the default permission sets on file system and database based resources such as configuration files and log files and database tables.

·         Data Protection in Storage & Transit: This category deals with protection of data both in storage and in transit. The nature of issues that are covered in this category include whether sensitive information such as social security numbers, user credentials or credit card information is being transmitted in the clear or stored as plaintext in the database. It is also important to ensure that all cryptographic primitives being used are well-known, well-documented and publicly scrutinized algorithms and that key lengths meet industry standards and best practices. For instance, ensure that the developers are using strong algorithms like AES (RSA for public key deployments) with key lengths of 128 bit (2048 bit for asymmetric keys) at a minimum. Similarly also ensure that cryptographically strong sources of randomness are being used to generate keys, session IDs and other such tokens. For instance, the use of the rand / Math.Random to generate a authentication token should be flagged as a flaw since these are easily guessable. Instead, the developers should use classes like SecureRandom or cryptographic APIs like Microsoft CAPI.

·         Authentication: Consider the lack of strong protocols to validate the identity of a user or component as part of this category. Other issues that must be considered include the possibility or potential for authentication attacks such as brute-force or dictionary based guessing attacks. If account lockouts are implemented, it is important to consider the potential for denial of service i.e. can an attacker lock out accounts permanently and most importantly can they lock out the administrative accounts. The quality or the lack of a password policy must also be reviewed for adherence to enterprise or industry requirements and best practices.

·         Authorization: The types of issues that are considered under this category include those dealing with the lack of or appropriate mechanisms to enforce access control on protected resources in the system. For instance, can a malicious user elevate his / her privilege by changing a authorization token, or can a business critical piece of data, such as the price of a product in an e-commerce application, be tampered by the attacker. One of the most common findings we discover is the so-called “admin token”. These are special tokens or flags which if passed to the application causes it to launch the administrative interface, disable all security checks or allow unfettered access in some form. The developers typically introduce these to aid in debugging and either forget to take them out from production systems or assume no one will find them. Authorization flaws typically result in either horizontal or vertical privilege escalation. These flaws represent the biggest category of problems we find in working with our clients.

·         User & Session Management: This category includes all those issues that deal with how a user’s session is managed within the application. Typical issues to look out for in this category include determining whether a session token can be replayed to impersonate the user, whether sessions time-out after an extended period of inactivity. Session isolation is also an important consideration. The reviewers must ensure that a user is only provided with information from within his / her own session and that he / she cannot intrude into the session of another user. As mentioned above it is also important to ensure that session tokens are random and not guessable. Further user management issues such as user provisioning and deprovisioning, password management and policies are also covered as part of this category.

·         Data Validation: This is the category responsible for the most well known bugs and flaws including buffer overflows, SQL injection and cross site scripting. The reviewers must ensure that all data that comes from outside the trust boundary[4][4] of a component is sanitized and validated. Data sanitization includes type, format, length and range checks. It is especially important to check for how the application deals with non-canonicalized data e.g. data that is UNICODE encoded. The code reviewers should check for use of output validation which is critical and recommended for dealing with problems such as cross-site scripting. Also if the application is to be internationalized or localized for a specific language, the impact on regular expression validators and the application in general must be verified. Auditors must scan for any instances of SQL queries being constructed dynamically using string concatenation of parameters obtained from the user. These represent the most common attack vector for SQL and other injection attacks. It is also important to ensure that any stored procedures being used do not operate in an unsafe manner for example, that they do not use string parameters and the exec call to execute other stored procedures.

·         Error Handling & Exception Management: This category is responsible for ensuring that all failure conditions such as errors and exceptions are dealt with in a secure manner. The nature of issues covered in this category range from detailed error messages, that lead to information disclosure, to how user friendly security error messages are. For instance do these messages clearly indicate the security and usability implications of their decision and whether they are provided with enough information to make that decision. The code reviewing team must ensure that exception handlers wrap all security significant operations such as database operations and cryptography. The reviewers must also ensure that page and application level exception handlers are set.

·         Auditing and Logging: The final category of issues discovered during code inspection is concerned with how information is logged for debugging and auditing purposes. For instance, are all security sensitive operations being logged to create an audit trail. This includes but is not restricted to failed and successful logins, impersonation, privilege elevation, change of log setting or clearing the log, cryptographic operations and session lifetime events. The review team must also ensure that log files cannot be modified, deleted or cleared by unauthorized users. Another common issue we find is when too much information is being logged leading to information disclosure of sensitive information. It is important to verify that the logging capabilities cannot be used in a resource exhaustion attack on the server through excessive logging without any form of quotas or log archiving policies. Finally, especially when logs can be viewed by administrators within the same application, the team must ensure that no cross-site scripting tags can be inserted into the log files.

The code inspection team may chose to review each section of code for all the categories above or alternatively apply each category to the entire code base. In our experience the latter tends to be more effective with larger code bases where the code review team is not attempting for 100% code coverage. On the other hand for smaller projects where the team will be reviewing every line of code it is usually more effective and efficient to use the former.

Once, the code inspection process has been completed and the findings documented, it is important to consider how to go about dealing with the resulting issues. At this stage it is important to be strategic rather than tactical. If possible patch solutions must be avoided. The findings must be reviewed to determine what the root causes are. For instance, if the team finds that the majority of the issues stem out of the fact that developers are not adhering to best practices and policies than chances are that they have not been made adequately aware of these and therefore efforts must be spent on dispersing this information. Similarly, if a majority of the flaws are related to authorization than training in role based access control or other authorization best practices might be in order. In our experience, patch solutions often tend to deal with specific issues and thus either lead to further problems or do not fix the core issue and hence the issue can be exploited through a different attack vector.

Summary

In our experience, code review including threat modeling and security code inspection is the only effective way to perform a through analysis of your application’s code base and find the maximum vulnerabilities and other security issues. This also represents an effective way to integrate security into your software development lifecycle. More importantly once fully understood and integrated into the development process these can not only be invaluable tools to improve quality but can also be optimized and made extremely efficient so that the overall impact on the lifecycle is not a significant overhead. As would seem only logical it makes sense to leverage any advantage one has over the attacker and your application’s design, architecture and source code is perhaps the biggest such advantage.