Difference between revisions of "Code Review Introduction"

From OWASP
Jump to: navigation, search
(The Basics: What we know we don’t know and what we know we know.)
(Added navigation to facilitate sequential reading online)
 
(27 intermediate revisions by 5 users not shown)
Line 1: Line 1:
[[OWASP Code Review Guide Table of Contents]]__TOC__
+
{{LinkBar
 +
  | useprev=PrevLink | prev=Code Review Guide History | lblprev=
 +
  | usemain=MainLink | main=OWASP Code Review Guide Table of Contents | lblmain=Table of Contents
 +
  | usenext=NextLink | next=Code Review Preparation | lblnext=Preparation
 +
}}
 +
__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 ==
 
== Introduction ==
 +
Code review is probably the single-most effective technique for identifying security flaws.  When used together with automated tools and manual penetration testing, code review can significantly increase the cost effectiveness of an application security verification effort.
  
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.
+
This guide does not prescribe a process for performing a security code review. Rather, this guide focuses on the mechanics of reviewing code for certain vulnerabilities, and provides limited guidance on how the effort should be structured and executed. OWASP intends to develop a more detailed process in a future version of this guide.
 
+
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 artefacts 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 its expected to do and who uses it.
+
  
Simple questions like:
+
Manual security code review provides insight into the “real risk” associated with insecure code. This is the single most important value from a manual approach.  A human reviewer can understand the context for certain coding practices, and make a serious risk estimate that accounts for both the likelihood of attack and the business impact of a breach.
  
“'''''What type/how sensitive is the data/Asset contained in the application?'''''”:
+
===Why Does Code Have Vulnerabilities?===
 +
MITRE has catalogued almost 700 different kinds of software weaknesses in their CWE project. These are all different ways that software developers can make mistakes that lead to insecurity. Every one of these weaknesses is subtle and many are seriously tricky. Software developers are not taught about these weaknesses in school and most do not receive any training on the job about these problems.
  
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?
+
These problems have become so important in recent years because we continue to increase connectivity and to add technologies and protocols at a shocking rate. Our ability to invent technology has seriously outstripped our ability to secure it.  Many of the technologies in use today simply have not received any security scrutiny.
  
“'''''Is the application internal or external facing?'''''”, “'''''Who uses the application, are they trusted users?'''”''
+
There are many reasons why businesses are not spending the appropriate amount of time on security. Ultimately, these reasons stem from an underlying problem in the software market. Because software is essentially a black-box, it is extremely difficult to tell the difference between good code and insecure code. Without this visibility, buyers won’t pay more for secure code, and vendors would be foolish to spend extra effort to produce secure code.
  
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!!”
+
One goal for this project is to help software buyers gain visibility into the security of software and start to effect change in the software market.  
  
''“'''Where does the application host sit'''?”''
+
Nevertheless, we still frequently get pushback when we advocate for security code review. Here are some of the (unjustified) excuses that we hear for not putting more effort into security:
  
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.
+
''“We never get hacked (that I know of), we don’t need security”''
  
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?
+
''“We have a firewall that protects our applications”''
  
'''''How important is this application to the enterprise?'''''”.
+
''"We trust our employees not to attack our applications" ''
  
Is the application of minor significance or a Tier A / Mission critical application, which the enterprise would fail without. 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.
+
Over the last 10 years, the team involved with the OWASP Code Review Project has performed thousands of application reviews, and found that every single application has had serious vulnerabilities. If you haven’t reviewed your code for security holes, the likelihood that your application has problems is virtually 100%.
  
A useful approach is to present the team with a checklist, which asks the relevant, questions pertaining to any web application.:
+
Still, there are many organizations that choose not to know about the security of their code. To them, we offer Rumsfeld’s cryptic explanation of what we actually know. If you’re making informed decisions to take risk in your enterprise, we fully support you. However, if you don’t even know what risks you are taking, you are being irresponsible both to your shareholders and your customers.
  
 +
''"...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." ''    - Donald Rumsfeld
  
===The Checklist ===
+
===What is Security Code Review? ===
 +
Security code review is the process of auditing the source code for an application to verify that the proper security controls are present, that they work as intended, and that they have been invoked in all the right places. Code review is a way of ensuring that the application has been developed so as to be “self-defending” in its given environment.
  
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:
+
Security code review is a method of assuring secure application developers are following secure development techniques. A general rule of thumb is that a penetration test should not discover any additional application vulnerabilities relating to the developed code after the application has undergone a proper security code review.
  
* Authentication
+
All security code reviews are a combination of human effort and technology support. At one end of the spectrum is an inexperienced person with a text editor.  At the other end of the scale is a security expert with an advanced static analysis tool. Unfortunately, it takes a fairly serious level of expertise to use the current application security tools effectively.
* 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:
+
Tools can be used to perform this task but they always need human verification. Tools do not understand context, which is the keystone of security 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 to determine if it is a real issue, if it is actually exploitable, and calculate the risk to the enterprise.
  
<u>http://www.owasp.org/docroot/owasp/misc/designreviewchecklist.doc</u>
+
Human reviewers are also necessary to fill in for the significant blind spots where automated tools simply cannot check.
  
The checklist is a good barometer for the level of security the developers have attempted or thought of.
+
{{LinkBar
 +
  | useprev=PrevLink | prev=Code Review Guide History | lblprev=
 +
  | usemain=MainLink | main=OWASP Code Review Guide Table of Contents | lblmain=Table of Contents
 +
  | usenext=NextLink | next=Code Review Preparation | lblnext=Preparation
 +
}}
  
 
[[Category:OWASP Code Review Project]]
 
[[Category:OWASP Code Review Project]]

Latest revision as of 09:41, 9 September 2010

«««« Main
(Table of Contents)
»»Preparation»»

Contents


Introduction

Code review is probably the single-most effective technique for identifying security flaws. When used together with automated tools and manual penetration testing, code review can significantly increase the cost effectiveness of an application security verification effort.

This guide does not prescribe a process for performing a security code review. Rather, this guide focuses on the mechanics of reviewing code for certain vulnerabilities, and provides limited guidance on how the effort should be structured and executed. OWASP intends to develop a more detailed process in a future version of this guide.

Manual security code review provides insight into the “real risk” associated with insecure code. This is the single most important value from a manual approach. A human reviewer can understand the context for certain coding practices, and make a serious risk estimate that accounts for both the likelihood of attack and the business impact of a breach.

Why Does Code Have Vulnerabilities?

MITRE has catalogued almost 700 different kinds of software weaknesses in their CWE project. These are all different ways that software developers can make mistakes that lead to insecurity. Every one of these weaknesses is subtle and many are seriously tricky. Software developers are not taught about these weaknesses in school and most do not receive any training on the job about these problems.

These problems have become so important in recent years because we continue to increase connectivity and to add technologies and protocols at a shocking rate. Our ability to invent technology has seriously outstripped our ability to secure it. Many of the technologies in use today simply have not received any security scrutiny.

There are many reasons why businesses are not spending the appropriate amount of time on security. Ultimately, these reasons stem from an underlying problem in the software market. Because software is essentially a black-box, it is extremely difficult to tell the difference between good code and insecure code. Without this visibility, buyers won’t pay more for secure code, and vendors would be foolish to spend extra effort to produce secure code.

One goal for this project is to help software buyers gain visibility into the security of software and start to effect change in the software market.

Nevertheless, we still frequently get pushback when we advocate for security code review. Here are some of the (unjustified) excuses that we hear for not putting more effort into security:

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

“We have a firewall that protects our applications”

"We trust our employees not to attack our applications"

Over the last 10 years, the team involved with the OWASP Code Review Project has performed thousands of application reviews, and found that every single application has had serious vulnerabilities. If you haven’t reviewed your code for security holes, the likelihood that your application has problems is virtually 100%.

Still, there are many organizations that choose not to know about the security of their code. To them, we offer Rumsfeld’s cryptic explanation of what we actually know. If you’re making informed decisions to take risk in your enterprise, we fully support you. However, if you don’t even know what risks you are taking, you are being irresponsible both to your shareholders and your customers.

"...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." - Donald Rumsfeld

What is Security Code Review?

Security code review is the process of auditing the source code for an application to verify that the proper security controls are present, that they work as intended, and that they have been invoked in all the right places. Code review is a way of ensuring that the application has been developed so as to be “self-defending” in its given environment.

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

All security code reviews are a combination of human effort and technology support. At one end of the spectrum is an inexperienced person with a text editor. At the other end of the scale is a security expert with an advanced static analysis tool. Unfortunately, it takes a fairly serious level of expertise to use the current application security tools effectively.

Tools can be used to perform this task but they always need human verification. Tools do not understand context, which is the keystone of security 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 to determine if it is a real issue, if it is actually exploitable, and calculate the risk to the enterprise.

Human reviewers are also necessary to fill in for the significant blind spots where automated tools simply cannot check.


«««« Main
(Table of Contents)
»»Preparation»»