OWASP - User contributions [en]

Security Code Review Coverage

2007-11-19T18:25:33Z

Jlaliberte: capitalization, spelling, rewording, spacing

[[OWASP Code Review Guide Table of Contents]]__TOC__

==Transactional Analysis: ==

''“For every input there will be an equal and opposite output (Well sort of)”''

A major part of actually performing a Secure Code inspection is performing a transactional analysis. An application takes inputs and produces output of some kind.

All input to the code needs to be defined.
For example, input can come from:
* Browser input (HTTP)
* Cookies
* Other Entity (machines/external processes).
* Property files
* ….
Input changes the state of an application. Attackers use input streams to attack applications. Would systems lacking an input mechanism be 100% secure? Probably not.

The input mechanisms, the path the input takes throughout the application, and the output based on the transformed input needs to be documented.

Transactional analysis is of paramount importance when performing code inspection. Any input stream that is overlooked may be a potential door for an attacker.

Transactional analysis includes any cookie or state information passed between the client and server and not just information inputted by the user, the payload.

Take into account any potential errors that can occur in the application for a given input. Are the errors handled properly by the application?

Transactional Analysis includes dynamic and static data flow analysis;
where and when variables are set, how the variable are used throughout the workflow, and how attributes of objects and parameters might affect other data within the program. Transactional analysis can help to determine if the parameters, method calls, and data exchange mechanisms implement the required security.

All transactions within the application need to be identified and analyzed along with the relevant security functions they invoke.
The areas that are covered during transaction analysis are:
* Authentication
* Authorization
* Cookie Management
* Data/Input Validation from all external sources.
* Error Handling /Information Leakage
* Logging /Auditing
* Cryptography (Data at rest and in transit)
* Secure Code Environment
* Session Management (Login/Logout)

== General principles (What to look for) ==

For each of the areas above a reviewer must look at the following principles in the enforcement of the requirement:

===Authentication: ===

* Ensure all internal and external connections (user and entity) go through an appropriate and adequate form of authentication. Be assured that this control cannot be bypassed.
* Ensure all pages enforce the requirement for authentication.
* Ensure that whenever authentication credentials or any other sensitive information is passed, only accept the information via the HTTP “POST” method and will not accept it via the HTTP “GET” method.
* Any page deemed by the business or the development team as being outside the scope of authentication should be reviewed in order to assess any possibility of security breach.
* Ensure that authentication credentials do not traverse the wire in clear text form.
* Ensure development/debug backdoors are not present in production code.

===Authorization: ===

* Ensure that there are authorization mechanisms in place.
* Ensure that the application has clearly defined the user types and the rights of said users.
* Ensure there is a least privilege stance in operation.
* Ensure that the Authorization mechanisms work properly, fail securely, and cannot be circumvented.
* Ensure that authorization is checked on every request.
* Ensure development/debug backdoors are not present in production code.

===Cookie Management: ===

* Ensure that sensitive information is not compromised.
* Ensure that unauthorized activities cannot take place via cookie manipulation.
* Ensure that encryption is used properly.
* Ensure secure flag is set to prevent accidental transmission over “the wire” in a non-secure manner.
* Determine if all state transitions in the application code properly check for the cookies and enforce their use.
* Ensure the session data is being validated.
* Ensure cookie contains as little private information as possible.
* Ensure entire cookie should be encrypted if sensitive data is persisted in the cookie.
* Define all cookies being used by the application, their name and why they are needed.

=== Data/Input Validation: ===

* Ensure that a DV mechanism is present.
* Ensure all input that can (and will) be modified by a malicious user such as http headers, input fields, hidden fields, drop down lists & other web components are properly validated.
* Ensure that the proper length checks on all input exist.
* Ensure that all fields, cookies, http headers/bodies & form fields are validated.
* Ensure that the data is well formed and contains only known good chars is possible.
* Ensure that the data validation occurs on the server side.
* Examine where data validation occurs and if a centralized model or decentralized model is used.
* Ensure there are no backdoors in the data validation model.
* '''''Golden Rule: All external input, no matter what it is, is examined and validated.'''''

===Error Handling/Information leakage: ===

* Ensure that all method/function calls that return a value have proper error handling and return value checking.
* Ensure that exceptions and error conditions are properly handled.
* Ensure that no system errors can be returned to the user.
* Ensure that the application fails in a secure manner.
* Ensure resources are released if an error occurs.

===Logging/Auditing: ===

* Ensure that no sensitive information is logged in the event of an error.
* Ensure the payload being logged is of a defined maximum length and that the logging mechanism enforces that length.
* Ensure no sensitive data can be logged; E.g. cookies, HTTP “GET” method, authentication credentials.
* Examine if the application will audit the actions being taken by the application on behalf of the client particularly and data manipulation/Create, Update, Delete (CUD) operations.
* Ensure successful & unsuccessful authentication is logged.
* Ensure application errors are logged.
* Examine the application for debug logging with the view to logging of sensitive data.

===Cryptography: ===

* Ensure no sensitive data is transmitted in the clear, internally or externally.
* Ensure the application is implementing known good cryptographic algorithms.

===Secure Code Environment: ===

* Examine the file structure, are any components that should not be directly accessible available to the user.
* Examine all memory allocations/de-allocations.
* Examine the application for dynamic SQL and determine is vulnerable to injection.
* Examine the application for “main()” executable functions and debug harnesses/backdoors
* Search for commented out code, commented out test code, which may contain sensitive information.
* Ensure all logical decisions have a default clause.
* Ensure no development environment kit is contained on the build directories.
* Search for any calls to the underlying operating system or file open calls and examine the error possibilities.

=== Session management: ===

* Examine how and when a session is created for a user, unauthenticated and authenticated.
* Examine the session ID and verify is a complex enough to fulfill requirements regarding strength.
* Examine how sessions are stored: E.g. In a database, in memory etc.
* Examine how the application tracks sessions.
* Determine the actions the application takes if an invalid session ID occurs.
* Examine session invalidation.
* Determine how multithreaded/multi-user session management is performed.
* Determine the session HTTP inactivity timeout.
* Determine how the log-out functionality functions.

== Understand what you are reviewing: ==

Many modern applications are developed on frameworks. These frameworks provide the developer less work to do as the framework does much of the “House Keeping”. So the objects developed by the development team shall extend the functionality of the framework.
<u>It is here that the knowledge of a given framework and language which the framework and application is implemented in is of paramount importance</u>. Much of the transactional functionality may not be visible in the developers code and handled in “Parent” classes.

The analyst must be aware and knowledgeable of the underlying framework

'''For example:'''

===Java: ===
In struts the ''struts-config.xml'' and the ''web.xml'' files are the core points to view the transactional functionality of an application.

<?xml version="1.0" encoding="ISO-8859-1" ?>
<!DOCTYPE struts-config PUBLIC
"-//Apache Software Foundation//DTD Struts Configuration 1.0//EN"
"http://jakarta.apache.org/struts/dtds/struts-config_1_0.dtd">
<struts-config>

<form-beans>
<form-bean name="login" type="test.struts.LoginForm" />
</form-beans>

<global-forwards>
</global-forwards>

<action-mappings>
<action
path="/login"
type="test.struts.LoginAction" >
<forward name="valid" path="/jsp/MainMenu.jsp" />
<forward name="invalid" path="/jsp/LoginView.jsp" />
</action>
</action-mappings>

<plug-in className="org.apache.struts.validator.ValidatorPlugIn">
<set-property property="pathnames"
value="/test/WEB-INF/validator-rules.xml, /WEB-INF/validation.xml"/>
</plug-in>
</struts-config>

The ''struts-config.xml'' file contains the action mappings for each HTTP request while the ''web.xml'' file contains the deployment descriptor.

'''Example''': The struts framework has a validator engine, which relies on regular expressions to validate the input data. The beauty of the validator is that no code has to be written for each form bean. (Form bean is the java object which received the data from the HTTP request) . The validator is not enabled by default in struts. To enable the validator a plug-in must be defined in the <plug-in> section of struts-config.xml in Red above. The property defined tells the struts framework where the custom validation rules are defined (validation.xml) and a definition of the actual rules themselves (validation-rules.xml).

Without a proper understanding of the struts framework and by simply auditing the Java code one would not see any validation being executed and one does not see the relationship between the defined rules and the java functions.

The action mappings in Blue define the action taken by the application upon receiving a request. Here, above we can see that when the URL contains /login the '''''LoginAction''''' shall be called. From the action mappings we can see the transactions the application performs when external input is received.

===.NET: ===
ASP.NET/ IIS applications use an optional XML-based configuration file named ''web.config'', to maintain application configuration settings. This covers issues such as authentication, authorisation, Error pages, HTTP settings, debug settings, web service settings etc..

Without knowledge of these files a transactional analysis would be very difficult and not accurate.

Optionally, you may provide a file ''web.config'' at the root of the virtual directory for a Web application. If the file is absent, the default configuration settings in ''machine.config'' will be used. If the file is present, any settings in ''web.config'' will override the default settings.

Example of the web.config file:

<authentication mode="Forms">
<forms name="name"
loginUrl="url"
protection="Encryption"
timeout="30" path="/" >
requireSSL="true|"
slidingExpiration="false">
<credentials passwordFormat="Clear">
<user name="username" password="password"/>
</credentials>
</forms>
<passport redirectUrl="internal"/>
</authentication>

OK so from this config file snippet we can see that:

'''authentication mode''': The default authentication mode is ASP.NET forms-based authentication.

'''loginUrl: '''Specifies the URL where the request is redirected for logon if no valid authentication cookie is found.

'''protection: '''Sepcifies that the cookie is encrypted using 3DES or DES but DV is not performed on the cookie. Beware of plaintext attacks!!

'''timeout: '''Cookie expiry time in minutes

The point to make here is that many of the important security settings are not set in the code per se but in the framework configuration files.
Knowledge of the framework is of paramount importance when reviewing framework-based applications.

[[Category:OWASP Code Review Project]]

Security Code Review in the SDLC

2007-11-19T17:59:41Z

Jlaliberte: capitalization, extra spaces, paren

[[OWASP Code Review Guide Table of Contents]]__TOC__

==Preface==

Code reviews vary widely in their level of formality. Reviews can be as informal as inviting a friend to help look for a hard to find bug, and they can be as formal as a software inspection process with trained teams, assigned roles and responsibilities, and a formal metric and quality tracking program.

In ''Peer Reviews in Software,'' Karl Wiegers lists seven review processes from least to most formal:

# Ad hoc review
# Passaround
# Pair programming
# Walkthrough
# Team review
# Inspection

== Mature Secure Code Review (SCR) Model ==

Throughout the SDLC there are points at which an application security consultant should get involved. These points, "touch points" can be used to investige the status of the code being developed from a security standpoint. The reason for interviening at regular intervals is that potential issues can be detected early on in the development life cycle and hence total cost of ownership (TCO) is less in the long term.

'''Waterfall SDLC exmaple'''
# Requirements definition
## Functional specification
# Design
## Detailed design specification
# Development
## Coding
## Unit tests
# Test
## Functional testing
## System testing
## Integration testing
## UAT (User acceptance testing)
# Deployment
## Change control
# Maintenance

[[Category:OWASP Code Review Project]]

== Minimal Resource Available Code Review for Web Applications Model ==

Very often, risk managers are tasked to manually code review large applications with minimal time and resources. This guide will focus on streamlining the manual code review process and outline the bare minimal essentials that are required for review.

'''Manual Code Review should at LEAST focus on:'''
#Authorization
#Access Control
#Input Validation
#Error Handling
#Session Management
#Form Keys or Frequent Session Rotation (for CSRF defense)
#Proper Application Logging

[[Category:OWASP Code Review Project]]

Steps and Roles

2007-11-19T17:54:49Z

Jlaliberte: spelling, paragraph spacing

[[OWASP Code Review Guide Table of Contents]]__TOC__

==Roles==

Code reviews are carried out by personnel in four roles: author, moderator, reader, and scribe. There are typically reviewers who are simply inspectors, focused on finding defects in the code, who do not fit in any of the four roles. Depending on the size of your inspection team and the formality of your inspection process, some people may serve in multiple roles at the same time. However, if you have a large enough team, it is useful to assign each role to a different person so each person can focus on their duties.

#'''Moderator''': The Moderator is the key role in a code review. The moderator is responsible for selecting a team of reviewers, scheduling the code review meeting, conducting the meeting, and working with the author to ensure that necessary corrections are made to the reviewed document.

#'''Author''': The Author wrote the code that is being reviewed. The author is responsible for starting the code review process by finding a Moderator. The role of Author must be separated from that of Moderator, Reader, or Recorder to ensure the objectivity and effectiveness of the code review. However, the Author serves an essential role in answering questions and making clarifications during the review and making corrections after the review.

#'''Reader''': The Reader presents the code during the meeting by paraphrasing it in his own words. It's important to separate the role of Reader from Author, because it's too easy for an author to explain what he meant the code to do instead of explaining what it actually does. The reader's interpretation of the code can reveal ambiguities, hidden assumptions, poor documentation and style, and other errors that the Author would not be likely to catch on his own.

#'''Scribe''': The Scribe records all issues raised during the code review. Separating the role of Scribe from the other roles allows the other reviewers to focus their entire attention on the code.

==Steps==

Code reviews consist of the following four steps:

#'''Initialization''': The Author informs a Moderator that a deliverable will be ready for inspection in the near future. The Moderator selects a team of inspectors and assigns roles to them. The Author and the Moderator together prepare a review package consisting of the code to be reviewed, documentation, review checklists, coding rules, and other materials such as the output of static analysis tools. The Moderator will announce the time, place, and duration for the code review meeting.

#'''Preparation''': After receiving the review package, the inspectors study the code individually to search for defects. Preparation should take about as long as the duration of the meeting. Some less formal code review techniques skip the preparation phase.

#'''Meeting''': The Moderator initiates the meeting, then the Reader describes the code to the participants. After each segment of code is presented, reviewers will bring up any issues they found during Preparation or discovered during the meeting. The interaction between reviewers during the meeting will usually bring up issues that were not discovered during the Preparation step. The Scribe notes each defect with enough detail for the Author to address it afterwards. It is the responsibility of the Moderator to keep the meting focused on defects, ensuring that the participants do not attempt to produce solutions during the meeting instead. Some less formal code review steps skip the meeting phase, choosing instead to e-mail the code to one or more reviewers who return comments without ever meeting as a group.

#'''Corrections''': The Author addresses the defects recorded during the meeting, and the Moderator checks the corrections to ensure that all problems are resolved. If the number of defects raised was large, the Moderator may decide to schedule a review of the revised code.
[[Category:OWASP Code Review Project]]

Code Review Introduction

2007-11-19T17:49:06Z

Jlaliberte: spelling and other various corrections

[[OWASP Code Review Guide Table of Contents]]__TOC__

==Preface==

This document is not a “How to perform a Secure Code review” walkthrough but more a guide on how to perform a successful review. Knowing the mechanics of code inspection is a half the battle but I’m afraid people is the other half.

A proper code review will not only identify vulnerabilities, but will assess which vulnerabilities are at the greatest risk for exploitation.

This document describes how to make the most of a secure code review.

== Introduction ==

The only possible way of developing secure software and keeping it secure going into the future is to make security part of the design. When cars are designed safety is considered and is now a big selling point for people buying a new car, “How safe is it?” would be a question a potential buyer may ask, also look at the advertising referring to the “Star” rating for safety a brand/model of car has.

Unfortunately the software industry is not as evolved and hence people still buy software without paying any regard to the security aspect of the application.

Every day more and more vulnerabilities are discovered in popular applications, which we all know and use and even use for private transactions over the web.

I’m writing this document not from a puristic point of view. Not everything you may agree with but from experience it is rare that we can have the luxury of being a purist in the real world.

Many forces in the business world do not see value in spending a proportion of the budget in security and factoring some security into the project timeline.

The usual one liners we hear in the wilderness:

''“We never get hacked (that I know of), we don’t need security” ''

''“We never get hacked, we got a firewall”.''

''Question: “How much does security cost”? Answer: “How much shall no security cost”?''

''"Not to know is bad; not to wish to know is worse."''

Code inspection is a fairly low-level approach to securing code but is very effective.

==The Basics: What we know we don’t know and what we know we know. ==
<br>
''"...we know, there are known knowns; there are things we know we know. We also know there are known unknowns; that is to say we know there are some things we do not know. But there are also unknown unknowns -- the ones we don't know we don't know''."
<br> - Donald Rumsfeld.
===What is Secure Code Review? ===

Secure code review is the process of auditing code for an application on a line by line basis for its security quality. Code review is a way of ensuring that the application is developed in an appropriate fashion so as to be “self defending” in its given environment.

Secure Code review is a method of assuring secure application developers are following secure development techniques. A general rule of thumb is that a pen test should not discover any additional application vulnerabilities relating to the developed code after the application has undergone a proper secure code review.

Secure code review is a manual process. It is labor intensive and not very scalable but it is accurate if performed by humans (and not tools, so far).

Tools can be used to perform this task but they always need human verification. Tools do not understand context, which is the keystone of secure code review. Tools are good at assessing large amounts of code and pointing out possible issues but a person needs to verify every single result and also figure out the false positives and the false negatives.

There are many source code review tool vendors. None have created a “silver bullet” at a reasonable cost. Vendor tools that are effective cost upwards around $60,000 USD per developer seat.

Code review can be broken down into a number of discrete phases.

# Discovery (Pre Transaction analysis).
# Transactional analysis.
# Post transaction analysis.
# Procedure peer review.
# Reporting & Presentation.

===Laying the ground work ===

(Ideally the reviewer should be involved in the design phase of the application, but this is not always possible so we assume the reviewer was not.)

There are two scenarios to look at.

# The security consultant was involved since project inception and has guided and helped integrate security into the SDLC.
# The security consultant is brought into the project near project end and is presented with a mountain of code, has no insight into the design, functionality or business requirements.

So we’ve got 100K lines of code for secure code inspection, how do we handle this?

The most important step is collaboration with developers. Obtaining information from the developers is the most efficient method for performing an accurate code review.

Performing code review can feel like an audit, and everybody hates being audited. The way to approach this is to create an atmosphere of collaboration between the reviewer, the development team and any stakeholders. Portraying an image of an advisor and not a policeman is very important if you wish to get full co-operation from the development team.

“''Help me help you''” is the approach and ideal that needs to be communicated.

===Discovery: Gathering the information ===

As mentioned above talking to developers is arguably the most accurate and the quickest way of gaining insight into the application.

A culture of collaboration between the security analyst and the development team is important to establish.

Design documents, business requirements, functional specifications, and any other documentation relating to the code being reviewed can be helpful.

'''Before we start:'''

The reviewer(s) need to be familiar with:
# '''Code''': The language used, the features and issues of that language from a security perspective. The issues one needs to look out for and best practices from a security and performance perspective.
# '''Context''': They need to be familiar with the application being reviewed. All security is in context of what we are trying to secure. Recommending military standard security mechanisms on an application that vends apples would be over-kill, and out of context. What type of data is being manipulated or processed and what would the damage to the company be if this data was compromised. Context is the “''Holy Grail''” of secure code inspection and risk assessment…we’ll see more later.
# '''Audience''': From 2 (above) we need to know the users of the application, is it externally facing or internal to “Trusted” users. Does this application talk to other entities (machines/services)? Do humans use this application?
# '''Importance''': The availability of the application is also important. Shall the enterprise be affected in any great way if the application is “bounced” or shut down for a significant or insignificant amount of time?

===Context, Context, Context ===

The context in which the application is intended to operate is a very important issue in establishing potential risk.

Defining context should provide us with the following information:
*Establish the importance of the application to the enterprise.
*Establish the boundaries of the application context.
*Establish the trust relationships between entities.
*Establish potential threats and possible countermeasures.

So we can establish something akin to a threat model. Take into account where our application sits, what it's expected to do and who uses it.

Simple questions like:

“'''''What type/how sensitive is the data/asset contained in the application?'''''”:

This is a keystone to security and assessing possible risk to the application. How desirable is the information? What effect would it have on the enterprise if the information were compromised in any way?

“'''''Is the application internal or external facing?'''''”, “'''''Who uses the application; are they trusted users?'''”''

This is a bit of a false sense of security as attacks take place by internal/trusted users more often than is acknowledged. It does give us context that the application ''should'' be limited to a finite number of identified users but its not a guarantee that these users shall all behave properly.

''“'''Where does the application host sit'''?”''

Users should not be allowed past the DMZ into the LAN without being authenticated. Internal users also need to be authenticated. No authentication = no accountability and a weak audit trail.

If there are internal and external users, what are the differences from a security standpoint? How do we identify one from another. How does authorisation work?

“'''''How important is this application to the enterprise?'''''”.

Is the application of minor significance or a Tier A / Mission critical application, without which the enterprise would fail? Any good web application development policy would have additional requirements for different applications of differing importance to the enterprise. It would be the analyst’s job to ensure the policy was followed from a code perspective also.

A useful approach is to present the team with a checklist, which asks the relevant, questions pertaining to any web application.

===The Checklist ===

Defining a generic checklist which can be filled out by the development team is of high value is the checklist asks the correct questions in order to give us context. The checklist should cover the “Usual Suspects” in application security such as:

* Authentication
* Authorization
* Data Validation (a''nother “holy grail”'')
* Session management
* Logging
* Error handling
* Cryptography
* Topology (where is this app in the network context).

An example can be found:

<u>http://www.nemozzang.com/pub/Guide%20Line/OWASP/designreviewchecklist.doc</u>

The checklist is a good barometer for the level of security the developers have attempted or thought of.

[[Category:OWASP Code Review Project]]

Code Review Introduction

2007-11-19T17:33:28Z

Jlaliberte: cleanup preface paragraphs

[[OWASP Code Review Guide Table of Contents]]__TOC__

==Preface==

This document is not a “How to perform a Secure Code review” walkthrough but more a guide on how to perform a successful review. Knowing the mechanics of code inspection is a half the battle but I’m afraid people is the other half.

A proper code review will not only identify vulnerabilities, but will assess which vulnerabilities are at the greatest risk for exploitation.

This document describes how to make the most of a secure code review.

== Introduction ==

The only possible way of developing secure software and keeping it secure going into the future is to make security part of the design. When cars are designed safety is considered and is now a big selling point for people buying a new car, “How safe is it?” would be a question a potential buyer may ask, also look at the advertising referring to the “Star” rating for safety a brand/model of car has.

Unfortunately the software industry is not as evolved and hence people still buy software without paying any regard to the security aspect of the application.

Every day more and more vulnerabilities are discovered in popular applications, which we all know and use and even use for private transactions over the web.

I’m writing this document not from a puristic point of view. Not everything you may agree with but from experience it is rare that we can have the luxury of being a purist in the real world.

Many forces in the business world do not see value in spending a proportion of the budget in security and factoring some security into the project timeline.

The usual one liners we hear in the wilderness:

''“We never get hacked (that I know of), we don’t need security” ''

''“We never get hacked, we got a firewall”.''

''Question: “How much does security cost”? Answer: “How much shall no security cost”?''

''"Not to know is bad; not to wish to know is worse."''

Code inspection is a fairly low-level approach to securing code but is very effective.

==The Basics: What we know we don’t know and what we know we know. ==
<br>
''"...we know, there are known knowns; there are things we know we know. We also know there are known unknowns; that is to say we know there are some things we do not know. But there are also unknown unknowns -- the ones we don't know we don't know''."
<br> - Donald Rumsfeld.
===What is Secure Code Review? ===

Secure code review is the process of auditing code for an application on a line by line basis for its security quality. Code review is a way of ensuring that the application is developed in an appropriate fashion so as to be “self defending” in its given environment.

Secure Code review is a method of assuring secure application developers are following secure development techniques. A general rule of thumb is that a pen test should not discover any additional application vulnerabilities relating to the developed code after the application has undergone a proper secure code review.

Secure code review is a manual process. It is labour intensive and not very scalable but it is accurate if performed by humans (and not tools, so far).

Tools can be used to perform this task but they always need human verification. Tools do not understand context, which is the keystone of secure code review. Tools are good at assessing large amounts of code and pointing out possible issues but a person needs to verify every single result and also figure out the false positives and worse again the false negatives.

There are many source code review tool vendors. None have created a “silver bullet” at a reasonable cost. Vendor tools that are effective cost upwards around $60,000 USD per developer seat.

Code review can be broken down into a number of discrete phases.

# Discovery (Pre Transaction analysis).
# Transactional analysis.
# Post transaction analysis.
# Procedure peer review.
# Reporting & Presentation.

===Laying the ground work ===

(Ideally the reviewer should be involved in the design phase of the application, but this is not always possible so we assume the reviewer was not.)

There are two scenarios to look at.

# The security consultant was involved since project inception and has guided and helped integrate security into the SDLC.
# The security consultant is brought into the project near project end and is presented with a mountain of code, has no insight into the design, functionality or business requirements.

So we’ve got 100K lines of code for secure code inspection, how do we handle this?

The most important step is collaboration with developers. Obtaining information from the developers is the most time saving method for performing an accurate code review in a timely manner.

Performing code review can feel like an audit, and everybody hates being audited. The way to approach this is to create an atmosphere of collaboration between the reviewer, the development team & vested interests. Portraying an image of an advisor and not a policeman is very important if you wish to get full co-operation from the development team.

“''Help me help you''” is the approach and ideal that needs to be communicated.

===Discovery: Gathering the information ===

As mentioned above talking to developers is arguably the most accurate and definitely the quickest way of gaining insight into the application.

A culture of collaboration between the security analyst and the development team is important to establish.

Other artifacts required would be design documents, business requirements, functional specifications and any other relating information.

'''Before we start:'''

The reviewer(s) need to be familiar with:
# '''Code''': The language used, the features and issues of that language from a security perspective. The issues one needs to look out for and best practices from a security and performance perspective.
# '''Context''': They need to be familiar with the application being reviewed. All security is in context of what we are trying to secure. Recommending military standard security mechanisms on an application that vends apples would be over-kill, and out of context. What type of data is being manipulated or processed and what would the damage to the company be if this data was compromised. Context is the “''Holy Grail''” of secure code inspection and risk assessment…we’ll see more later.
# '''Audience''': From 2 (above) we need to know the users of the application, is it externally facing or internal to “Trusted” users. Does this application talk to other entities (machines/services)? Do humans use this application?
# '''Importance''': The availability of the application is also important. Shall the enterprise be affected in any great way if the application is “bounced” or shut down for a significant or insignificant amount of time?

===Context, Context, Context ===

The context in which the application is intended to operate is a very important issue in establishing potential risk.

Defining context should provide us with the following information:
*Establish the importance of application to enterprise.
*Establish the boundaries of the application context.
*Establish the trust relationships between entities.
*Establish potential threats and possible countermeasures.

So we can establish something akin to a threat model. Take into account where our application sits, what it's expected to do and who uses it.

Simple questions like:

“'''''What type/how sensitive is the data/asset contained in the application?'''''”:

This is a keystone to security and assessing possible risk to the application. How desirable is the information? What effect would it have on the enterprise if the information were compromised in any way?

“'''''Is the application internal or external facing?'''''”, “'''''Who uses the application; are they trusted users?'''”''

This is a bit of a false sense of security as attacks take place by internal/trusted users more often than is acknowledged. It does give us context that the application ''should'' be limited to a finite number of identified users but its not a guarantee that these users shall all behave properly.

''“'''Where does the application host sit'''?”''

Users should not be allowed past the DMZ into the LAN without being authenticated. Internal users also need to be authenticated. No authentication = no accountability and a weak audit trail.

If there are internal and external users, what are the differences from a security standpoint? How do we identify one from another. How does authorisation work?

“'''''How important is this application to the enterprise?'''''”.

Is the application of minor significance or a Tier A / Mission critical application, without which the enterprise would fail? Any good web application development policy would have additional requirements for different applications of differing importance to the enterprise. It would be the analyst’s job to ensure the policy was followed from a code perspective also.

A useful approach is to present the team with a checklist, which asks the relevant, questions pertaining to any web application.

===The Checklist ===

Defining a generic checklist which can be filled out by the development team is of high value is the checklist asks the correct questions in order to give us context. The checklist should cover the “Usual Suspects” in application security such as:

* Authentication
* Authorization
* Data Validation (a''nother “holy grail”'')
* Session management
* Logging
* Error handling
* Cryptography
* Topology (where is this app in the network context).

An example can be found:

<u>http://www.nemozzang.com/pub/Guide%20Line/OWASP/designreviewchecklist.doc</u>

The checklist is a good barometer for the level of security the developers have attempted or thought of.

[[Category:OWASP Code Review Project]]

Code Review Introduction

2007-11-19T17:20:28Z

Jlaliberte: capitalization error

[[OWASP Code Review Guide Table of Contents]]__TOC__

==Preface==

This document is not a “How to perform a Secure Code review” walkthrough but more a guide on how to perform a successful review. Knowing the mechanics of code inspection is a half the battle but I’m afraid people is the other half.

To perform a proper code review, to give value to the client from a risk perspective and not from an academic or text book perspective we must understand what we are reviewing.

Applications may have faults but the client wants to know the “real risk” and not necessarily what the security textbooks say.

Albeit there are real vulnerabilities in real applications out there and they pose real risk but how do we define real risk as opposed to best practice?

This document describes how to get the most out of a secure code review. What is important when managing an engagement with a client and how to keep your eye on the ball the see the “wood from the trees”.

== Introduction ==

The only possible way of developing secure software and keeping it secure going into the future is to make security part of the design. When cars are designed safety is considered and is now a big selling point for people buying a new car, “How safe is it?” would be a question a potential buyer may ask, also look at the advertising referring to the “Star” rating for safety a brand/model of car has.

Unfortunately the software industry is not as evolved and hence people still buy software without paying any regard to the security aspect of the application.

Every day more and more vulnerabilities are discovered in popular applications, which we all know and use and even use for private transactions over the web.

I’m writing this document not from a puristic point of view. Not everything you may agree with but from experience it is rare that we can have the luxury of being a purist in the real world.

Many forces in the business world do not see value in spending a proportion of the budget in security and factoring some security into the project timeline.

The usual one liners we hear in the wilderness:

''“We never get hacked (that I know of), we don’t need security” ''

''“We never get hacked, we got a firewall”.''

''Question: “How much does security cost”? Answer: “How much shall no security cost”?''

''"Not to know is bad; not to wish to know is worse."''

Code inspection is a fairly low-level approach to securing code but is very effective.

==The Basics: What we know we don’t know and what we know we know. ==
<br>
''"...we know, there are known knowns; there are things we know we know. We also know there are known unknowns; that is to say we know there are some things we do not know. But there are also unknown unknowns -- the ones we don't know we don't know''."
<br> - Donald Rumsfeld.
===What is Secure Code Review? ===

Secure code review is the process of auditing code for an application on a line by line basis for its security quality. Code review is a way of ensuring that the application is developed in an appropriate fashion so as to be “self defending” in its given environment.

Secure Code review is a method of assuring secure application developers are following secure development techniques. A general rule of thumb is that a pen test should not discover any additional application vulnerabilities relating to the developed code after the application has undergone a proper secure code review.

Secure code review is a manual process. It is labour intensive and not very scalable but it is accurate if performed by humans (and not tools, so far).

Tools can be used to perform this task but they always need human verification. Tools do not understand context, which is the keystone of secure code review. Tools are good at assessing large amounts of code and pointing out possible issues but a person needs to verify every single result and also figure out the false positives and worse again the false negatives.

There are many source code review tool vendors. None have created a “silver bullet” at a reasonable cost. Vendor tools that are effective cost upwards around $60,000 USD per developer seat.

Code review can be broken down into a number of discrete phases.

# Discovery (Pre Transaction analysis).
# Transactional analysis.
# Post transaction analysis.
# Procedure peer review.
# Reporting & Presentation.

===Laying the ground work ===

(Ideally the reviewer should be involved in the design phase of the application, but this is not always possible so we assume the reviewer was not.)

There are two scenarios to look at.

# The security consultant was involved since project inception and has guided and helped integrate security into the SDLC.
# The security consultant is brought into the project near project end and is presented with a mountain of code, has no insight into the design, functionality or business requirements.

So we’ve got 100K lines of code for secure code inspection, how do we handle this?

The most important step is collaboration with developers. Obtaining information from the developers is the most time saving method for performing an accurate code review in a timely manner.

Performing code review can feel like an audit, and everybody hates being audited. The way to approach this is to create an atmosphere of collaboration between the reviewer, the development team & vested interests. Portraying an image of an advisor and not a policeman is very important if you wish to get full co-operation from the development team.

“''Help me help you''” is the approach and ideal that needs to be communicated.

===Discovery: Gathering the information ===

As mentioned above talking to developers is arguably the most accurate and definitely the quickest way of gaining insight into the application.

A culture of collaboration between the security analyst and the development team is important to establish.

Other artifacts required would be design documents, business requirements, functional specifications and any other relating information.

'''Before we start:'''

The reviewer(s) need to be familiar with:
# '''Code''': The language used, the features and issues of that language from a security perspective. The issues one needs to look out for and best practices from a security and performance perspective.
# '''Context''': They need to be familiar with the application being reviewed. All security is in context of what we are trying to secure. Recommending military standard security mechanisms on an application that vends apples would be over-kill, and out of context. What type of data is being manipulated or processed and what would the damage to the company be if this data was compromised. Context is the “''Holy Grail''” of secure code inspection and risk assessment…we’ll see more later.
# '''Audience''': From 2 (above) we need to know the users of the application, is it externally facing or internal to “Trusted” users. Does this application talk to other entities (machines/services)? Do humans use this application?
# '''Importance''': The availability of the application is also important. Shall the enterprise be affected in any great way if the application is “bounced” or shut down for a significant or insignificant amount of time?

===Context, Context, Context ===

The context in which the application is intended to operate is a very important issue in establishing potential risk.

Defining context should provide us with the following information:
*Establish the importance of application to enterprise.
*Establish the boundaries of the application context.
*Establish the trust relationships between entities.
*Establish potential threats and possible countermeasures.

So we can establish something akin to a threat model. Take into account where our application sits, what it's expected to do and who uses it.

Simple questions like:

“'''''What type/how sensitive is the data/asset contained in the application?'''''”:

This is a keystone to security and assessing possible risk to the application. How desirable is the information? What effect would it have on the enterprise if the information were compromised in any way?

“'''''Is the application internal or external facing?'''''”, “'''''Who uses the application; are they trusted users?'''”''

This is a bit of a false sense of security as attacks take place by internal/trusted users more often than is acknowledged. It does give us context that the application ''should'' be limited to a finite number of identified users but its not a guarantee that these users shall all behave properly.

''“'''Where does the application host sit'''?”''

Users should not be allowed past the DMZ into the LAN without being authenticated. Internal users also need to be authenticated. No authentication = no accountability and a weak audit trail.

If there are internal and external users, what are the differences from a security standpoint? How do we identify one from another. How does authorisation work?

“'''''How important is this application to the enterprise?'''''”.

Is the application of minor significance or a Tier A / Mission critical application, without which the enterprise would fail? Any good web application development policy would have additional requirements for different applications of differing importance to the enterprise. It would be the analyst’s job to ensure the policy was followed from a code perspective also.

A useful approach is to present the team with a checklist, which asks the relevant, questions pertaining to any web application.

===The Checklist ===

Defining a generic checklist which can be filled out by the development team is of high value is the checklist asks the correct questions in order to give us context. The checklist should cover the “Usual Suspects” in application security such as:

* Authentication
* Authorization
* Data Validation (a''nother “holy grail”'')
* Session management
* Logging
* Error handling
* Cryptography
* Topology (where is this app in the network context).

An example can be found:

<u>http://www.nemozzang.com/pub/Guide%20Line/OWASP/designreviewchecklist.doc</u>

The checklist is a good barometer for the level of security the developers have attempted or thought of.

[[Category:OWASP Code Review Project]]

Code Review Guide Foreword

2007-11-19T17:10:29Z

Jlaliberte:

==Foreword by [[User:Jeff Williams|Jeff Williams]], OWASP Chair==

Many organizations have realized that their code is not as secure as they may have thought. Now they're starting the difficult work of verifying the security of their applications. There are four basic techniques for analyzing code - automated scanning, manual penetration testing, static analysis, and manual code review.

This OWASP Guide is focused on the last of these techniques. Of course, all of these techniques have their strengths, weaknesses, sweet spots, and blind spots. Arguments about which technique is the best are like arguing whether a hammer or saw is more valuable when building a house. If you try to build a house with just a hammer, you'll do a terrible job.

==Why Code Review?==

Despite the many claims that code review is too expensive or time consuming, there is no question that it is the fastest and most accurate way to find and diagnose many security problems. There are also dozens of security problems that simply can't be found any other way.

Code review is also that only way to verify that security has been implemented correctly. By checking the code

TBD

==Getting Started==

TBD

==OWASP Guides==

The OWASP guides are intended to teach you how to use these techniques. But the fact that they are separate shouldn't be an indicator that they should be used alone. The [[Building Guide]] shows your project how to architect and build a secure application, this [[Code Review Guide]] tells you how to verify the security of your application's source code, and the [[Testing Guide]] shows you how to verify the security of your running application.

Security moves too fast for traditional books to be of much use. But OWASP's collaborative environment allows us to keep up to date. There are hundreds of contributors to the OWASP Guides and we make over a thousand updates to our materials every month. We're committed to making high quality application security materials available to everyone. It's the only way we'll ever make any real progress on application security as a software community.

==Call to Action==

If you're building software, I strongly encourage you to get familiar with the security guidance in this document. If you find errors, please add a note to the discussion page or make the change yourself. You'll be helping thousands of others who use this guide.

Please consider [[Membership|joining us]] as an individual or corporate member so that we can continue to produce materials like this code review guide and all the other great projects at OWASP.

Thank you to all the past and future contributors to this guide, your work will help to make applications worldwide more secure.

-- [[User:Jeff Williams|Jeff Williams]], OWASP Chair, October 17, 2007

__NOTOC__

[[Category:OWASP Code Review Project]]