OWASP Code Review Guide

User Manual:

Open the PDF directly: View PDF PDF.
Page Count: 191

DownloadOWASP Code Review Guide
Open PDF In BrowserView PDF
The Code Review Guide Book was first published in 2008 with the idea of having code
reviews and testing in one volume.
This version of the Code Review Guide Book was started in April of 2013 wiith a small
group of volunteers committeed to updating the information contained in the guide book.

Code Review Guide Book
v. 2.0 ALPHA
OWASP Foundation

2013 Edition
© 2013 OWASP Foundation
This document is released under the Creatvive Commons ATtribution-­ShareAlike 3.0
license. For any reuse or distribution, you must make clear to others the license terms of
this work.
Cover photo: Black wasp. http://www.morguefile.com/archive/display/604130
ISBN 978-1-304-58673-5

90000

9 781304 586735

Code Review Guide
Version 2.0 Pre-Alpha
Project Leaders Eoin Keary and Larry Conklin - November 7, 2013

The code review guide is currently at release version 1.1 and the second best selling OWASP
book in 2008. Many positive comments have been feedback regarding this initial version and
believe it’s a key enabler for the OWASP fight against software insecurity. It has even
inspired individuals to build tools based on its information. The combination of a book on
secure code review and tools to support such an activity is very powerful as it gives the
developer community a place to start regarding secure application development.
© 2013 OWASP Foundation
This document is licensed under the Creative Commons Attribution-ShareAlike 3.0 license

OWASP CODE REVIEW GUIDE - V2.0

1

Prefix
This document is a pre Alpha release to demonstrate where we are to date in relation to the OWASP
Code Review Guide. OWASP volunteers develop the Code Review Guide, people like you. The aim of
the guide is to help developers and code reviewers alike navigate a source code review and pinpoint
areas of weakness from a security standpoint.
If you would like to contribute please feel free to contact the team, we are not hard to find on the
interwar. If you have feedback, suggestions, or would like to send OWASP lots of donations to assist
in developing great documents, please also get in touch….
Thanks Eoin, Larry & Sam

OWASP CODE REVIEW GUIDE - V2.0

2

1.2.1 What is source code review and Static Analysis

12!

1.2.2 What is Code Review (Needs Content)

12!

1.2.3 Manual Review

12!

1.2.3.1 Choosing a static analysis tool

13!

1.2.4 Advantages of Code Review to Development Practices

14!

1.2.5 Why Code Review

17!

1.2.5.1 Scope and Objective of secure code review (Needs Content)

18!

1.2.6 We can’t hack ourselves secure

19!

1.2.7 360 Review: Coupling source code review and Testing / Hybrid Reviews (Needs
Content)
19!
1.2.8 Can static code analyzers do it all?

19!

2.1 The code review approach (Needs Content)

22!

2.1.1 Preparation and context

22!

2.1.2 Understanding Code layout/Design/Architecture (Needs Content)

27!

2.2 SDLC Integration (Needs Content)

27!

2.2.1 Deployment Models (Needs Content)

27!

2.2.1.1 Secure deployment configurations (Needs Content)

27!

2.2.1.2 Metrics and code review (Needs Content)

27!

2.2.1.3 Source and sink reviews (Needs Content)

27!

2.2.1.4 Code review coverage (Needs Content)

27!

2.2.1.5 Design Reviews (Needs Content)

27!

2.2.1.6 A Risk based approach to code review (Needs Content)

43!

2.2.2 Crawling Code

43!

2.2.2.1 Searching for Code in .NET

44!

2.2.2.2 Searching for Code in Java

51!

2.2.2.3 Searching for Code in Classic ASP

56!

2.2.2.4 Searching for Code in Javascript and AJAX

58!

2.2.2.5 Searching for Code in C++ and Apache

59!

OWASP CODE REVIEW GUIDE - V2.0

3

2.2.3 Code Reviews and Compliance (Needs Content)

61!

3.1 Reviewing code for Authentication controls (Needs Content)

62!

3.1.1 Forgot Password

62!

3.1.2 Authentication (Needs Content)

64!

3.1.3 CAPTCHA

64!

3.1.4 Out of Band Considerations (Needs Content)

67!

3.2 Reviewing code for Authorization weakness

67!

3.2.1 Checking authorization upon every request

68!

3.2.2 Reducing the attack surface (Needs Content)

69!

3.2.3 SSL/TLS Implementations

70!

3.2.4 Reviewing code for session handling

70!

3.2.5 Reviewing client side code (Needs Content)

73!

3.2.5.1 Javascript

73!

3.2.5.2 JSON (Needs Content)

74!

3.2.5.3 Content Security Policy (Needs Content)

74!

3.2.5.4 “Jacking”/Framing

74!

3.2.5.5 HTML 5? (Needs Content)

75!

3.2.5.6 Browser Defenses Policy (Needs Content)

75!

3.2.5.7 Etc… (Needs Content)

75!

3.2.6 Review code for input validation (Needs Content)

75!

3.2.6.1 Regex Gotchas (Needs Content)

75!

3.2.6.2 ESAPI (Needs Content)

75!

3.2.7 Review code for contextual encoding

76!

3.2.7.1 HTML Attribute

76!

3.2.7.2 HTML Entity

77!

3.2.7.3 Javascript Parameters

79!

3.2.7.4 JQuery (Needs Content)

81!

3.2.8 Reviewing file and resource handling code (Needs Content)

81!

OWASP CODE REVIEW GUIDE - V2.0

4

3.2.9 Resource Exhaustion - error handling (Needs Content)

81!

3.2.9.1 Native Calls (Needs Content)

81!

3.2.10 Reviewing logging code - Detective Security (Needs Content)

81!

3.2.11 Reviewing Error handling and Error messages

82!

3.2.12 Reviewing Security alerts (Needs Content)

99!

3.2.13 Reviewing for active defense

100!

3.2.14 Reviewing Secure Storage (Needs Content)

105!

3.2.15 Hashing & Salting - When, How, and Where

105!

4.1 Review Code for XSS

111!

4.2 Persistent - The Anti Pattern (Needs Content)

112!

4.2.1 .NET

112!

4.2.2 Java

115!

4.2.3 PHP

118!

4.2.4 Ruby (Needs Content)

118!

4.3 Reflected - The Anti Pattern (Needs Content)

119!

4.3.1 .NET

119!

4.3.2 Java

120!

4.3.3 PHP

121!

4.3.4 Ruby (Needs Content)

122!

4.4 Stored - The Anti Pattern (Needs Content)

122!

4.4.1 .NET (Needs Content)

122!

4.4.2 Java

122!

4.4.3 PHP (Needs Content)

123!

4.4.4 Ruby (Needs Content)

123!

4.5 DOM XSS

123!

4.6 JQuery Mistakes (Needs Content)

125!

4.7 Reviewing code for SQL Injection (Needs Content)

125!

4.7.1 PHP

125!

OWASP CODE REVIEW GUIDE - V2.0

5

4.7.2 Java

128!

4.7.3 .NET (Needs Content)

129!

4.7.4 HQL (Needs Content)

129!

4.8 The Anti Pattern

129!

4.8.1 PHP (Needs Content)

131!

4.8.2 Java (Needs Content)

132!

4.8.3 .NET (Needs Content)

132!

4.8.4 Ruby (Needs Content)

132!

4.8.5 Cold Fusion

132!

4.9 Reviewing code for CSRF Issues

132!

4.10 Transactional logic / Non idempotent functions / State Changing Functions
(Needs Content)
132!
4.11 Reviewing code for poor logic /Business logic/Complex authorization (Needs
Content)
133!
4.12 Reviewing Secure Communications (Needs Content)

133!

4.12.1 .NET Config

133!

4.12.2 Spring Config (Needs Content)

144!

4.12.3 HTTP Headers (Needs Content)

144!

4.13 Tech-Stack Pitfalls (Needs Content)

145!

4.14 Framework Specific Issues (Needs Content)

145!

4.14.1 Spring

145!

4.14.2 Structs (Needs Content)

148!

4.14.3 Drupal (Needs Content)

148!

4.14.4 Ruby on Rails (Needs Content)

148!

4.14.5 Django (Needs Content)

148!

4.14.6 .NET Security / MVC

148!

4.14.7 Security in ASP .NET applications

156!

4.14.7.1 Strongly Named Assemblies

157!

OWASP CODE REVIEW GUIDE - V2.0

6

4.14.7.1.1 Round Tripping

161!

4.14.7.1.2 How to prevent Round tripping

162!

4.14.7.2 Setting the right Configurations

162!

4.14.7.3 Authentication Options

166!

4.14.7.4 Code Review for Managed Code - .Net 1.0 and up

167!

4.14.7.5 Using OWASP Top 10 as your guideline

174!

4.14.7.6 Code review for Unsafe Code (C#)

178!

4.14.8 PHP Specific Issues (Needs Content)

180!

4.14.9 Classic ASP

180!

4.14.10 C# (Needs Content)

180!

4.14.11 C/C++ (Needs Content)

180!

4.14.12 Objective C (Needs Content)

180!

4.14.13 Java (Needs Content)

181!

4.14.14 Android (Needs Content)

183!

4.14.15 Coldfusion (Needs Content)

183!

4.14.16 CodeIgniter (Needs Content)

183!

OWASP CODE REVIEW GUIDE - V2.0

7

1.1 Forward
The OWASP Code Review guide is the result of initially contributing and leading the Testing Guide.
Initially, it was thought to place Code review and testing into the same guide; it seemed like a good
idea at the time. But the topic called security code review got too big and evolved into its own standalone guide.
Eoin Keary started the Code Review guide in 2006. This current version was started in April 2013 via
the OWASP Project Reboot initiative. The OWASP Code Review team consists of a small, but
talented, group of volunteers who should really get out more often.
It is common knowledge that more secure software can be produced and developed in a more cost
effective way when bugs are detected early on in the systems development lifecycle. Organizations
with a proper code review functions integrated into the software development lifecycle (SDLC)
produced remarkably better code from a security standpoint. Simply put "We can't hack ourselves
secure". Attackers have more time to fine vulnerabilities on a system than the time allocated to a
defender. Hacking our way secure amounts to an uneven battlefield; Asymmetric warfare, a losing
battle.
By necessity, this guide does not cover all languages; it mainly focuses on .NET and Java, but has a
little C/C++ and PHP thrown in also. However, the techniques advocated in the book can be easily
adapted to almost any code environment. Fortunately, the security flaws in web applications are
remarkably consistent across programming languages.

OWASP CODE REVIEW GUIDE - V2.0

8

1.2 Code Review Guide Introduction
Code review is probably the single-most effective technique for identifying security flaws early in the
system development lifecycle. 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 guidance on how
the effort should be structured and executed.
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 of a bug or vulnerability in code. Context requires human understanding of what is being
assessed. With appropriate context we can make a serious risk estimate that accounts for both the
likelihood of attack and the business impact of a breach. Correct categorization of vulnerabilities helps
with priority of remediation and fixing the right things as opposed to wasting time fixing everything.

Why Does Code Have Vulnerabilities?
MITRE has catalogued circa 800 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:

OWASP CODE REVIEW GUIDE - V2.0

9

''“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 and logical 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 helping ensure that the application has been
developed so as to be “self-defending” in its given environment.
Security code review is a method helping assure 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. - al least very few issues should be discovered.
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 (SAST) tool. Unfortunately, it takes a fairly serious level of
expertise to use the current application security tools effectively. They also don't understand dynamic
data /page flow or business logic. SAST tools are great for coverage and setting a minimum baseline.
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

OWASP CODE REVIEW GUIDE - V2.0

10

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.

OWASP CODE REVIEW GUIDE - V2.0

11

1.2.1 What is source code review and Static Analysis
What is Security Source Code Review?
Source code review is the practice of reviewing developed code for vulnerabilities. There are many
ways to review the security of an application and it is recommended to perform more than one method
to help ensure more assessment coverage. Penetration testing is great at finding certain bugs such as
technical signature or API based issues. Issues related to privacy, information leakage, denial of
service is more suited to code review. Source code review is also good practice as you are finding
issues early in the SDLC. Locating and fixing issues early in your SDLC makes it cheaper in terms of
effort and cost to remediate. It also empowers developers to understand security bugs at the source
code level such that they may not repeat the same mistakes.

What is static analysis?
Static Code Analysis is usually performed as part of a Source code review and is carried out at the
Implementation phase of SDLC. Static Code Analysis commonly refers to the running of static code
analysis tools that attempts to highlight possible vulnerabilities whiting the ‘static’ (non-running) source
code by using techniques such as Taint Analysis, Data Flow Analysis, Control Flow Graph, and
Lexical Analysis. When the analysis is performed on a runtime environment, it is referred to as
Dynamic Code Analysis. Ideally, such tools would automatically find security flaws with a high degree
of confidence that what is found is indeed a flaw. However, this is beyond the state of the art for many
types of application security flaws. Thus, such tools frequently serve as aids for an analyst to help
them zero in on security relevant portions of code so they can find flaws more efficiently, rather than a
tool that simply finds flaws automatically.

1.2.2 What is Code Review (Needs Content)
Lorem Ipsum

1.2.3 Manual Review
Manual review is sited when a risk-based approach to the code review is required. Risk based code
review works by.
1. Identification of the trust boundaries in the code. 2. Identification of data paths and storage classes.
3. Identification of authorization components. 4. Identification of authentication components. 5. Review
of input validation and encoding methods. 6. Review of logging components.


OWASP CODE REVIEW GUIDE - V2.0

12

Manual review is good for :
Data leakage detection Resource usage/exhaustion detection Business Logic review* Denial of
service Deep Dive review
Not so good for: Business Logic review* Level of coverage

1.2.3.1 Choosing a static analysis tool
Choosing a static analysis tool is a difficult task since there are a lot of choices. The comparison
charts below should help you decide which tool is right for you. This list is not exhaustive. The first
thing to do is to look to for a tool that supports the programming language of your choice. You also
have to decide whether you want a commercial tool or a free one. Usually the commercial tools have
more features and are more reliable than the free ones. The major commercial tools are equally
effective but their usability might differ. Next, there is the type of analysis you are looking for: Security
or Quality, Static or Dynamic analysis. You should also check the compatibility of the tool with your
programming environment. This was the easy part to narrow the choice down to a few tools. The next
step requires you to do some work since it is quite subjective. The best thing to do is to test a few
tools to see if you are satisfied with different aspects such as the user experience, the reporting of
vulnerabilities, the level of false positives, the customization, the customer support… The choice
should not be based on the number of features, but on the features that you need and how they could
be integrated in your SDLC. Also, before choosing the tool, the security expertise of the targeted users
should be clearly evaluated in order to choose an appropriate tool.

OWASP CODE REVIEW GUIDE - V2.0

13

Free static analysis tools

Commercial static analysis tools

1.2.4 Advantages of Code Review to Development Practices
Advantages of Code Review to Development Practices

OWASP CODE REVIEW GUIDE - V2.0

14

Integrating code review into your company’s development processes can have many benefits that will
be explored below. Some of these benefits depend upon the tools you use to perform code reviews,
how well that data is backed up, and how well those tools are used. The days of bringing developers
into a room and displaying code on a projector, whilst recording the review results on a printed copy
and behind us, many tools exist to make code review more efficient and to track the review
records/decisions. When the code review process is structured correctly, the act of reviewing code
can provide educational, audible and historical benefits to any organization.
The following provides a list of benefits that a code review procedure can add to development team.

Provides an Historical Record
If any developer has joined a company, or moved teams within a company, and had to maintain or
enhance a piece of code written years ago, one of the biggest frustrations can be the lack of context
the new developer has on the code. Various schools of opinion exist on code documentation, both
within the code (comments) and external to the code (design/functional docs, wikis, etc.), opinions
ranging from zero-documentation tolerance through to near-NASA level documentation where the size
of the documentation far exceeds the size of the code module.
If you think about the discussions that occur during a code review, many of these discussions, if
recorded, would provide valuable information (context) to module maintainers and new programmers.
From the writer describing the module along with some of their design decisions, to each reviewers
comments, stating why they think one SQL query should be restructured, or an algorithm changed,
there is a development story unfolding in front of the reviewers eyes which can (and should) be used
by future coders on the module, who are probably not involved in the review meetings.
Capturing those review discussions in a review tool automatically, and storing them for future
reference, will provide the development organization with a history of the changes on the module
which can be queried at a later time by new developers. These discussions can also contain links to
any architectural/functional/design/test specifications, bug or enhancement numbers, etc.

Verification Change has been tested
When a developer is about to submit code into the repository, how do you know they have sufficiently
tested it? Adding a description of the tests they have run (manually or automated) against the
changed code can give reviewers (and management) confidence that the change will work and not
cause any regressions. Also by declaring the tests the writer has ran against their change, the author
is allowing reviewers to suggest further testing that may have been missed by the author.

OWASP CODE REVIEW GUIDE - V2.0

15

In a development scenario where automated unit or component testing exists, the coding guidelines
can require the developer include those unit/component tests in the code review. This again allows
reviewers within this type of automated environment to ensure the correct unit/component tests are
going to be included in the environment, keeping the quality of the continuous integration.

Coding Education for Junior Developers
After you learn the basics of a language and read a few of the best practices book, how can you get
good on-the-job skills to learn more? Besides buddy coding (which rarely happens and is never cost
effective) and training sessions (brown bag sessions on coding, tech talks, etc.) the design and code
decisions discussed during a code review can be a learning experience for junior developers. Many
experienced developers may admit to this being a two way street, where new developers can come in
with new ideas or tricks that the older developers can learn from. Altogether this cross-pollination of
experience and ideas can only be beneficial to a development organization.

Familiarization with Code Base
When a new feature is developed, it is often integrated with the main code base, and here code
review can be a conduit for the wider team to learn about the new feature and how it's code will impact
the wider product. This helps prevent functional duplication where separate teams end up coding the
same small piece of functionality.
This also applies for development environments with silo'ed teams. Here the code review author can
reach out to other teams to gain their insight, and allow those other teams to review their code, and
everyone then learns a bit more about the company's code.

Pre-warning of Integration Clashes
In a busy code base there will be times (especially on core code) where multiple developers can be
writing code affecting the same module. Many people have had the experience of cutting the code
and running the tests, only to discover upon submission that some other change has modified the
functionality, requiring the author to recode and retest some aspects of their change. Spreading the
word on upcoming changes via code reviews gives a greater chance of a developer learning that a
change is about to impact their upcoming commit, and development timelines, etc., can be updated
accordingly.

Coding Guidelines Touch point

OWASP CODE REVIEW GUIDE - V2.0

16

Many development environments have coding guidelines which new code must adhere to. Coding
guidelines can take many forms, but it's worth pointing out that security guidelines can be a
particularly relevant touch point within a code review as unfortunately, though typically, the security
issues are understood by a subset of the development team. Therefore it can be possible to include
teams with various technical expertise into the code reviews, i.e. someone from the security team (or
that person in the corner who knows all the security stuff) can be invited as a technical subject expert
to the review to check the code from their particular angle. This is where the OWASP top 10 could be
enforced.

1.2.5 Why Code Review
Security code review aims to identify security flaws in the application related to its features and design
along with their exact root causes. With the increasing complexity of the applications and the advent
of new technologies the traditional way of testing may fail to detect all the security flaws present in the
applications. Thus, there is a need to understand the code of the application, external components,
technologies and configurations to be able to uproot all the flaws in different kinds of applications.
Such a deep dive into the application code also helps in determining exact mitigation techniques that
can be used to avert the security flaws. Let’s look in further detail of the benefits of security code
reviews.

Root cause analysis (Source to sink) – There are various reasons why security flaws manifest in
the application like lack of validation, parameter mishandling etc. During the assessment, the code is
thoroughly studied and such flaws are checked. In the process the exact root cause of the flaws are
captured. The complete data flow is traced and source to sink analysis is carried out. Source to sink
analysis here means to determine what are possible inputs to the application i.e. source, and how are
they being processed by it. Sink refers an insecure code pattern like for instance, a dynamic SQL
query. So, its analyzing what is the source (input) and the sink (vulnerable code pattern) for any
vulnerability. Consider a scenario where the source is a user input. It flows through the different
classes/components of the application and finally falls into a concatenated SQL query i.e. a sink, and
there is no proper validation being applied to it in the path. In this case the application will be
vulnerable to SQL Injection attack, as identified by the source to sink analysis. Such an analysis
exactly helps in understanding, which vulnerable inputs can lead to a possibility of an exploit in the
application.

Design Analysis – Design is an important aspect of security code review. The dynamic testing
approach does not involve design analysis, which also serves as it one of the limitations. The design

OWASP CODE REVIEW GUIDE - V2.0

17

flaws are difficult to uncover and requires knowledge of the entire data flow, input sources,
integrations and all the configurations of the application. Thus, the flaws related to mishandling of nonuser inputs and external integrations like server-to-server, which are unlikely to get covered in a
dynamic testing approach can be easily determined in a security code review. It covers the end-to-end
analysis of the application.

All Instances of the Vulnerabilities – Once a flaw is identified the next step of the security code
review process is to enumerate its all the possible instances present in the application. Through
security code reviews we can uncover insecure patterns present in all the files of the application.
Here, all insecure instances includes the list of different the insecure code patterns that lead to a
vulnerability and all of their occurrences. For instance, an application can be vulnerable to XSS
vulnerability because of use of unvalidated inputs in insecure display methods like scriplets,
response.write method etc. at several places.

Uncommon Security Flaws – Security code reviews is very specific to the application being
reviewed. It may highlight some flaws that are new or specific to the code implementation of the
application like insecure termination of execution flow, synchronization error etc. These flaws can only
be uncovered if we understand the application code flow and its logic well. Thus, security code review
is not just about scanning the code for set of unknown insecure code patterns. But it also involves
understanding the code implementation of the application and enumerating the flaws specific to it.

Limitations of Existing Security Controls – The application being reviewed might have been
designed with some security controls in place like centralized blacklist validation etc. or there could be
some inbuilt validation/security controls present in the application platform being used. These security
controls must be studied carefully to identify if they are foolproof. According to the implementation of
the control, the nature of attack or any specific attack vector that can be used to bypass it, must be
analyzed. Enumerating the weakness in the existing security control is another important aspect of the
security code reviews.

Specific Recommendations – The security code reviews also helps to come up with mitigation
techniques that can best suit the application, instead of a generic one.

1.2.5.1 Scope and Objective of secure code review (Needs Content)
Lorem Ipsum

OWASP CODE REVIEW GUIDE - V2.0

18

1.2.6 We can’t hack ourselves secure
We can’t hack ourselves secure. Penetration testing is generally a point in time test. As source code
changes the value of the findings of a penetration test degrade with time. There are also privacy,
compliance and stability and availability concerns, which are generally not covered by penetration
testing. Data information leakage in a cloud environment for example may not be discovered via a
penetration test.

1.2.7 360 Review: Coupling source code review and Testing / Hybrid Reviews (Needs
Content)
Lorem Ipsum

1.2.8 Can static code analyzers do it all?
Secure code review is a process of enumerating the flaws in the application. The flaws may exist in
the application due to insecure code, design or configuration. Out of which the flaws that arise due to
insecure code can be enumerated to a great extent through automated analysis, as most of them are
associated with insecure patterns.
Automated analysis can be carried out through any of the following three options:
Static Code analyzers or scanners
Custom scripts based on some pattern search
Open source tools
Though some scripts and some open source tools are efficient enough in finding insecure code
patterns but they often lack the capability of tracing the data flow. The use of static code analyzers or
the scanners, which identify the insecure code pattern along with source to the sink analysis, fulfill this
limitation.
With this we come to the next big question i.e. Can scanners i.e. static code analyzers do it
all?
Static code analyzers or the scanners are the most comprehensive options to automate the process of
review.
Some of the advantages of static code scanners are:
1. Reduction in manual efforts – Secure code review can be at times be a tedious process of
analyzing thousands of lines of code for a host of vulnerabilities. Moreover as the type of patterns to
be scanned almost remains common across application the task also tends to get a bit repetitive. In

OWASP CODE REVIEW GUIDE - V2.0

19

such a scenario, scanners play a big role is automating the process of searching the vulnerabilities
through big numbers of lines of code.
2. Time efficient – Scanners are must time efficient than manual reviews. In most cases the scanners
have proved to be much faster than manual process of reviewing the source code.
3. Finds all the instances of the vulnerabilities - Scanners are very effective in identifying all the
instances of a particular vulnerability with their exact location. This is really helpful for larger code
base where tracing for flaws in all the files is difficult.
4. Source to sink analysis – Most scanners today trace the code and identify the vulnerabilities
through source to sink analysis. They identify the inputs to the application and trace them thoroughly
throughout the code till they find them to be associated with any insecure code pattern. Such a source
to sink analysis helps the developers in understanding the flaws better as they get a complete root
cause analysis of the flaw.
5. Exhaustive coverage of vulnerability patterns – Most of the scanners have well-defined set of
test cases covering all the well-known vulnerabilities. Thus through the use of scanners we can
ensure that the code gets thoroughly checked for all the pre-defined set of flaws.
6. Elaborate reporting format – Scanners provide a detailed report on the observed vulnerabilities
with exact code snippets, risk rating and complete description of the vulnerabilities. This helps the
development teams to easily understand the flaws and implement necessary controls.
Limitations of static code analyzers:
1. Business Logic Flaws remain untouched – The flaws that are related to application’s logic,
transactions, and specific sensitive data remain untouched by the scanners. The security controls that
needs to be implemented in the application specific its features and design are often not pointed by
the scanners. This stands as a biggest limitation of the static code analyzers.
2. Limited scope – Static code analyzers are often designed for specific frameworks or certain set of
vulnerable patterns, they fail to address the issues not covered in their search pattern repository. So
the scanners often fail in catching up the vulnerabilities of the new versions of the framework that
keeps coming up.
3. Custom validations - Most of the static analyzers tool miss out the custom validations added in the
application while identifying the flaws. These could include blacklist or whitelist validation present in
the application before the input sources. It could also mean the customization added by the
developers to the existing design frameworks and inbuilt framework based API, the scanners that go
by pattern based search usually miss out in understanding such intricate details of the code.

OWASP CODE REVIEW GUIDE - V2.0

20

4. Design flaws – Design flaws are lessen known issues and static code analyzers often focus more
on the code than the design. Mainly if the application design is custom built it becomes challenging for
the scanners to trace the code flow.
5. Application specific recommendations – Scanners usually provide a generic solution and do not
point out application specific code changes. If the solutions are customized as per the design and the
feasibility of the application it will be clearer to the developers and require less code change.
Parameters to be considered for commercial scanners:
1. Source to sink analysis Capabilities
2. Support for frameworks
3. Ability to understand customized code or validations
4. Coverage of Business logic cases specially the ones related to authorization
5. Option to customize the pattern search

OWASP CODE REVIEW GUIDE - V2.0

21

2 Methodology (Missing Content)
We need to add content that introducing readers to what the methodology is.

2.1 The code review approach (Needs Content)
Lorem Ipsum

2.1.1 Preparation and context
Laying the Groundwork
In order to effectively review a code baseline, it is critical that the review team understands the
business purpose of the application and the most critical business impacts. This will guide them in
their search for serious vulnerabilities. The team should also identify the different threat agents, their
motivation, and how they could potentially attack the application.
All this information can be assembled into a high-level threat model of the application that represents
all of information that is relevant to application security. The goal for the reviewer is to verify that the
key risks have been properly addressed by security controls that work properly and are used in all the
right places. In some cases the threat model will already be created, in other cases the reviewers
might need to draft one up a threat model.
Ideally the reviewer should be involved in the design phase of the application, but this is almost never
the case. However regardless of the size of the code change, the engineer initiating the code review
should direct reviewers to any relevant architecture or design documents. The easiest way to do this
is to include a link to the documents (assuming they're stored in an online document repository) in the
initial e-mail, or in the code review tool if that is supported.
Performing code review can feel like an audit, and most developers hate being audited. The way to
approach this is to create an atmosphere of collaboration between the reviewer, the development
team, the business representatives, and any other vested interests. Portraying the image of an
advisor and not a policeman is very important if you wish to get full co-operation from the development
team.
Before We Start
The reviewer(s) need to be familiar with:
1. Code: The language(s) 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.

OWASP CODE REVIEW GUIDE - V2.0

22

2. Context: The working of 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.
3. Audience: The intended 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?
4. Importance: The size of the consequences of failure. Shall the enterprise be affected in any great
way if the application cannot perform its functions as intended?

Discovery: Gathering the Information
The reviewers will need certain information about the application in order to be effective. The
information should be assembled into a threat model that can be used to prioritize the review.
Frequently, this information can be obtained by studying design documents, business requirements,
functional specifications, test results, and the like. However, in most real-world projects, the
documentation is significantly out of date and almost never has appropriate security information. If the
development organization has procedures and templates for architecture and design documents, the
reviewer can suggest updates to ensure security is considered at these phases.
One of the most effective ways to get started is to talk with the developers and the lead architect for
the application. This does not have to be a long meeting; it could be a whiteboard session for the
development team to share some basic information about the key security considerations and
controls. A walkthrough of the actual running application is very helpful to give the reviewers a good
idea about how the application is intended to work. Also, a brief overview of the structure of the
codebase and any libraries used can help the reviewers get started.
If the information about the application cannot be gained in any other way, then the reviewers will
have to spend some time doing reconnaissance and sharing information about how the application
appears to work by examining the code.

Context, Context, Context
Security code review is not simply about reviewing code. It’s important to remember that the reason
that we review code is to ensure that the code adequately protects the information and assets it has
been entrusted with, such as money, intellectual property, trade secrets, lives, or data.

OWASP CODE REVIEW GUIDE - V2.0

23

The context in which the application is intended to operate is a very important issue in establishing
potential risk. If reviewers do not understand the business context, they will not be able to find the
most important risks and may focus on issues that are inconsequential to the business.
As preparation for a security code review, a high-level threat model should be prepared which
includes the relevant information. This process is described more fully in a later section, but the major
areas are listed here:
• Threat Agents
• Attack Surface (including any public and backend interfaces)
• Possible Attacks
• Required Security Controls (both to stop likely attacks and to meet corporate policy)
• Potential Technical Impacts
• Important Business Impacts

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 controls.
The reviewers can use simple questions like the following to gather this information from the
development team:
- “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, and 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 it’s not a guarantee that these users shall all behave properly. Even if a
application is sitting behind a firewall, defense in depth can be considered when the data is sensitive.

OWASP CODE REVIEW GUIDE - V2.0

24

- “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 authorization 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 developers with a checklist, which asks the relevant questions
pertaining to any web application.

The Checklist
Defining a generic checklist that can be filled out by the development team is of high value, if the
checklist asks the correct questions in order to give us context. The checklist is a good barometer for
the level of security the developers have attempted or thought of. If security code review becomes a
common requirement, then this checklist can be incorporated into a development procedure so that
the information is always available to code reviewers. The checklist should cover the most critical
security controls and vulnerability areas such as:

• Data Validation
• Authentication
• Session Management
• Authorization
• Cryptography
• Error Handling
• Logging
• Security Configuration
• Network Architecture

OWASP CODE REVIEW GUIDE - V2.0

25

Category

Vulnerable.Area

Facts.to.ANALYSE
1.#Are#there#backdoor/unexposed#business#logic#classes?

Presence.of.backdoor.parameters/functions/files

Code.Flow.–.Division.of.code.based.
on.MVC

Placement.of.checks

1.#Are#security#checks#placed#before#processing#inputs?

Insecure.Data.Binding.Mechanism

1.#Check#if#unexposed#instance#variables#are#present#in#form#objects#that#get#bound#to#user#inputs.#If#present,#
check#if#they#have#default#values.
2.#Check#if#unexposed#instance#variables#present#in#form#objects#that#get#bound#to#user#inputs.#If#present,#
check#if#they#get#initialized#before#form#binding.

Insecure.authentication.and.access.control.logic

Design
Authentication.and.Access.Control.
Mechanism

2.#Are#there#unused#configurations#related#to#business#logic?
3.#If#request#parameters#are#used#to#identify#business#logic#methods,#is#there#a#proper#mapping#of#user#
privileges#and#methods/actions#allowed#to#them?

Redundant.configuration

1.#Is#the#placement#of#authentication#and#authorization#check#correct?
2.#Is#there#execution#stopped/terminated#after#for#invalid#request?#I.e.#when#authentication/authorization#
check#fails?
3.#Are#the#checks#correct#implemented?#Is#there#any#backdoor#parameter?
4.#Is#the#check#applied#on#all#the#required#files#and#folder#within#web#root#directory?
1.#Is#there#any#default#configuration#like#AccessG#ALL?
2.#Does#the#configuration#get#applied#to#all#files#and#users?
3.#Incase#of#container#managed#authentication#G#Is#the#authentication#based#on#web#methods#only?
4.#Incase#of#container#managed#authentication#G#Does#the#authentication#get#applied#on#all#resources?

Insecure.Session.management

1.#Does#the#design#handle#sessions#securely?

Weak.Password.Handling

1.#Is#Password#Complexity#Check#enforced#on#the#password?
2.#Is#password#stored#in#an#encrypted#format?
3.#Is#password#disclosed#to#user/written#to#a#file/logs/console?

Data.Access.Mechanism

Presence.of.sensitive.data.in.configuration/code.files
1.#Are#database#credentials#stored#in#an#encrypted#format?
Presence/support.for.different.insecure.data.sources.and.
their.related.flaws
1.#Does#the#design#support#weak#datastores#like#flat#files

Centralized.Validation.and.
Interceptors

Weakness.in.any.existing.security.control

Entry.Points

Insecure.Data.handling.and.validation

1.#Does#the#centralized#validation#get#applied#to#all#requests#and#all#the#inputs?
2.#Does#the#centralized#validation#check#block#all#the#special#characters?
3.#Does#are#there#any#special#kind#of#request#skipped#from#validation?
4.#Does#the#design#maintain#any#exclusion#list#for#parameters#or#features#from#being#validated?
1.#Are#all#the#untrusted#inputs#validated?

Elevated.privilege.levels

1.#Is#the#data#sent#on#encrypted#channel?##Does#the#application#use#HTTPClient#for#making#external#
connections?
2.#Does#the#design#involve#session#sharing#between#components/modules?#Is#session#validated#correctly#on#
both#ends?
3.#Does#the#design#use#any#elevated#OS/system#privileges#for#external#connections/commands?

External.API’s.used

Known.flaws.present.in.3rd.party.APIs/functions

1.#Is#there#any#known#flaw#in#API's/Technology#used?#For#eg:#DWR

Inbuilt.Security.Controls

Common.Security.Controls

1.#Does#the#design#framework#provide#any#inbuilt#security#control?#Like#<%:#%>#in#ASP.NET#MVC.#
2.#Are#are#there#any#flaw/weakness#in#the#existing#inbuilt#control?
3.#Are#all#security#setting#enabled#in#the#design?

Architecture
External.Integrations

Insecure.data.transmission

Configuration

OWASP CODE REVIEW GUIDE - V2.0

26

2.1.2 Understanding Code layout/Design/Architecture (Needs Content)
Lorem Ipsum

2.2 SDLC Integration (Needs Content)
Lorem Ipsum

2.2.1 Deployment Models (Needs Content)
Lorem Ipsum

2.2.1.1 Secure deployment configurations (Needs Content)
Lorem Ipsum

2.2.1.2 Metrics and code review (Needs Content)
Lorem Ipsum

2.2.1.3 Source and sink reviews (Needs Content)
Lorem Ipsum

2.2.1.4 Code review coverage (Needs Content)
Lorem Ipsum

2.2.1.5 Design Reviews (Needs Content)
Introduction

This project highlights some of the vital areas of design security. We are all aware of “secure coding”
and practice it to great extent while developing applications. But do we give equal attention to –
“Secure Design”? Most of us would probably say, NO. Design level flaws are lesser-known concepts

OWASP CODE REVIEW GUIDE - V2.0

27

but their presence is a very big risk to the applications. Such flaws are hard to find in static or dynamic
application scans and instead requires deep understanding of application architecture and layout to
uncover them manually. With increasing business needs the complexities in application design and
architecture are also increasing. There is a rise in the use of custom design techniques and diverse
technologies in the applications today, which makes the need for design reviews imperative. This
project focuses on highlighting some important secure design principles that developers and
architects must adapt to build a secure application design. With the help of some design flaws we will
see the areas of design that are exposed to security risks and what measures can be taken to avoid
them in our design. It also includes mitigation techniques that can be implemented in the applications
to prevent them.

Understanding the design

- What is an application design?
A design is a blueprint of an application; it lays a foundation for its development. It illustrates the
layout of the application and identifies different application components needed for it. It is a structure
that determines execution flow of the application. Most of the application designs are based on a
concept of MVC. In such designs different components interact with each other in an ordered
sequence to serve any user request.

- Why should be review the design?
Design review should be an integral part of secure software development process. If the application is
reviewed for security at the design level many inherent backdoors can be uncovered. Design reviews
also help to implementing the security requirements in a better way.

Methodology
The methodology to be followed for design reviews is explained below:

OWASP CODE REVIEW GUIDE - V2.0

28

Collec&on(of(
Design(
Documents(

Design(study(

Design(Analysis(

Propose(
Security(
Requirements(

Design(
Finaliza&on(

Discussion(with(
the(team(

Recommend(
Design(Changes(

- Collection of Design Documents:
This phase involves collecting required information of the proposed design. It would involve all kinds
of documentation maintained by the development team about the design like flow charts, sequence
diagrams, class diagrams etc. Requirements documents are also needed to understand the objective
of the proposed design.

- Design Study:
In this phase the design is thoroughly studied mainly with respect to the data flow, different application
components and their interactions, data handling etc. This is achieved through manual analysis and
discussions with the design or technical architect’s team. The design and the architecture of the
application must be understood thoroughly to analyse vulnerable areas that can lead to security
breaches in the application. The key areas of the design that must be considered during threat
analysis are given below.

• Data Flow/Code Layout
• Access control
• Existing/Built-in Security controls
• Entry points of non-user inputs
• Integrations with external services
• Location of configurations file and data sources
• Add-ons and customization present (in case of built-in design framework)

This will help in identifying the trust boundaries for an application and thus aid in taking decisions
about the vulnerabilities and their risk levels posed to the application.

OWASP CODE REVIEW GUIDE - V2.0

29

- Design Analysis:
After understanding the design the next phase is to analyse the threats for the design. This phase
involves “threat modeling” the design.
The threats must be identified for different design areas that were identified in the previous step. It
involves observing the design from an attacker’s perspective and uncovering the backdoors and
insecure areas present in it. The analysis can be done broadly on the basis of 2 important criteria:
1. Insecure Implementation – This would mean the design has a loophole, which can lead to a
security breach in the application for instance, insecure reference to business logic functions.
2. Lack of secure implementation – This would mean the design has not incorporated secure
practices. For instance, in connection to external server different security requirements to protect
confidentiality and integrity of the data are not present.
Similar instances are listed below to illustrate the points that should be broadly considered while
analysing different design areas:
• Data Flow a.

Are user inputs used to directly reference a business logic class/function

b.

Is there a data binding flaw?

c.

Does it expose any backdoor parameter to invoke business logic?

d. Is the execution flow of the application correct?

• Authentication and access control a.

Does the design implement access control for all the files?

b.

Does it handle session securely?

c.

Is there SSO, does it leave any backdoor?

• Existing/built-in Security Controls a.

Weakness in any existing security control

b.

Is the placement of the security controls correct?

• Architecture –

OWASP CODE REVIEW GUIDE - V2.0

30

a.

Is there validation for all input sources?

b.

Is the connection to external servers secure?

• Configuration/code files and datastores a.

Are sensitive data present in configuration files?

b.

Does it support any insecure data source?

A detailed checklist is available here - Excel Doc was here at the end of this activity we get a list of
threats or insecure areas applicable to the design.

- Propose Security Requirements:
After analysing the insecure areas in the design in this step a list of security requirements
corresponding to them must be created. Requirements are high level changes or additions to be
incorporated in the design, for instance: Validate the inputs fetched from the webservice response
before processing them. Any protection that is needed for resolving the vulnerable area identified in
the design would go as a security requirement for the design. This phase involves listing all the
security requirements for the design along with security risk associated with them. This risk-based
approach would help the development teams in prioritizing the security requirements.

- Recommend Design Changes:
In this phase every security requirement must be associated with a security control. A security control
best suited for the design is proposed and documented. These security controls are an elaborate view
of the security requirements. Here, we would identify exact changes or additions to be incorporated in
the design that are needed to meet any requirement or mitigate a threat. The changes or controls
recommended for the design should be clear and detailed, as given in the instances below:
a.

Elaborate validation strategy with respect to:

b.

Identifying right application component like servlet filters, interceptors, validator classes etc.

c.

Placement of check c. Validation mechanism

d.

Use of 3rd party security API’s or inbuilt design features of the frameworks

e.

Encryption techniques

f.

Design Patterns And so on depending on the control to be built in the design.

OWASP CODE REVIEW GUIDE - V2.0

31

- Discussion with the design team:
The list of security requirements and proposed controls must be then discussed with the development
teams. The queries of the teams should be addressed and feasibility of incorporating the controls
must be determined. Exceptions, if any must be taken into account and alternate recommendations
should be proposed. In this phase a final agreement on the security controls is achieved.

- Design Finalization:
The final design incorporated by the development teams must be reviewed again and finalized for
further development process.

Design flaws
This section describes some of the important design flaws that can leave a backdoor in the
application to access it without authentication or manipulate its business logic. We will understand
such flaws and secure design recommendations in detail.

- Business Logic Decision
During testing it is crucial to identify the key parameters related to business logic and understand how
application handles them. This section will focus on insecure business logic decisions that are based
on such parameters. Two such cases are listed below, it is important to look for such scenarios in the
application while testing.

1. Use of non-editable controls – Applications may use the values of non-editable controls, drop-down
menus, hidden fields or query string parameters for business logic processing. If such fields contain
values like the type of the user, nature of the request, status of the transaction, etc. the attackers will
get a chance to manipulate them and perform unauthorized operations. The application developers
must understand that such fields are non-editable only in the context of the proxy tool. The attackers
can easily modify their values using a proxy editor tool and try to manipulate business logic.

2. Business logic decision based on presence or absence of certain parameters - This is especially
observed in ASP.NET applications where there is provision to make the server side controls
hidden/invisible for certain users. However, in most cases it has been observed that if the users add
the parameters corresponding to the UI elements that are kept hidden/invisible to them into the
request, they are able to change the behavior of the server side logic. Consider a scenario where only

OWASP CODE REVIEW GUIDE - V2.0

32

admin user can change password of other users of the system, as a result the field to enter username
is only made visible to the admin user. However, if a normal a user tries to add username parameter
in the request he/she will be able to trick the server in believing that the request has come from an
admin user and try to change password of other users. Thus there exists a hole in such applications
where the server side behavior can be influenced with request parameters. Users can perform
unauthorized operations in the application by supplying the values for the inputs fields that are hidden
from them. Secure Design Recommendation:

• The application must not expose such parameters to the users.
• If they are exposed, the application must not rely on request parameters for logical decisions. It
mustmaintain a separate copy of such values at the server side and use the same for business logic
processing.
• Apply proper authorization checks on the server side for all transactions, wherever necessary. Do
not depend on presence of a user input for such decisions.

- Business Logic Invocation Technique
In most of the design techniques the request parameters or the URL’s serve as sole factors to
determine the processing logic. In such a scenario the elements in the request, which are used for
such identifications, may be subject to manipulation attacks to obtain access to restricted resources or
pages in the application.
Consider a design below; here the business logic class is identified based on a configuration file that
keeps the mapping of the request URL and the business logic class i.e. action class.

OWASP CODE REVIEW GUIDE - V2.0

33

What is the flaw?
A flaw in such a design could be unused configurations present in the configuration file. Such
configurations that are not exposed as valid features in the application and could serve as a potential
backdoor to it. An unused configuration present in the configuration file of the application is shown
below:

OWASP CODE REVIEW GUIDE - V2.0

34

Observe that the “TestAction” has an insecure logic to delete records from the system. This can act as
a potential backdoor to the application.

Consider another scenario
In the some designs request parameters are used to identify business logic methods. In the figure
shown below a request parameter named “event” is used to identify and invoke the corresponding
event handling methods of the business logic/action class.

OWASP CODE REVIEW GUIDE - V2.0

35

What is the flaw?
Here, the user can attempt to invoke the methods of the events that are not visible to the user.

Secure Design Recommendation:
The applications must ensure to:
• Remove ALL redundant/test/unexposed business logic configurations from the file
• Apply necessary authorization check before processing business logic method
• Maintain a mapping of method/class/view names with the privilege level of the users, wherever
applicable and restrict access of the users to restricted URLs/methods/views.

Review Criteria
Understand the business logic invocation technique used in the design of any application. Check if the
user inputs are directly (i.e. without any restriction) used to determine any of the following elements
(as applicable):

• Business logic class
• Method names

OWASP CODE REVIEW GUIDE - V2.0

36

• View component

Data Binding Technique
Another popular feature seen in most of the design frameworks today is data binding, where the
request parameters get directly bound to the variables of the corresponding business/command
object. Binding here means that the instance variables of such classes get automatically initialized
with the request parameter values based on their names. Consider a sample design given below;
observe that the business logic class binds the business object with the request parameters.

What is the flaw?
The flaw in such design is that the business objects may have variables that are not dependent on the
request parameters. Such variables could be key variables like price, max limit, role etc. having static
values or dependent on some server side processing logic. A threat in such scenarios is that an
attacker may supply additional parameters in request and try to bind values for unexposed variable of
business object class. As illustrated in the figure below, the attacker sends an additional “price”
parameter in the request and binds with the unexposed variable “price” in business object, thereby
manipulating business logic.

OWASP CODE REVIEW GUIDE - V2.0

37

Secure Design Recommendation
• An important point to be noted here is that the business/form/command objects must have only
those instance variables that are dependent on the user inputs.
• If additional variables are present those must not be vital ones like related to the business rule for
the feature.
• In any case the application must accept only desired inputs from the user and the rest must be
rejected or left unbound. And initialization of unexposed of variables, if any must take place after the
binding logic.

Review Criteria
Review the application design and check it is incorporates a data binding logic. In case it does, check
if business objects/beans that get bound to the request parameters have unexposed variables that are
meant to have static values. If such variables are initialized before the binding logic this attack will
work successfully.

Placement of Security Controls
Placement of security checks is a vital area of review in an application design. Incorrect placements
can render the applied security controls null and void. So, it is important to study the application
design and spot the correctness of such checks in the overall execution flow of the design. Most of the

OWASP CODE REVIEW GUIDE - V2.0

38

application designs are based on the concept of Model-View-Controller (MVC). They have a central
controller, which listens to all incoming request and delegates control to appropriate form/business
processing logic. And ultimately the user is rendered with a view. In such a layered design, when
there are many entities involved in processing a request, developers often go wrong in placing the
security controls at the right place. Most application developers feel “view” is the right place to have
the security checks like authentication check etc.

Views

BROWSER
Views

User

Applicati
on

What is the flaw?
It thus seems logical that if you restrict the users at the page/view level they won’t be able to perform
any operation in the application. But what if instead of requesting for a page/view an unauthorized
user tries to request for an internal action like to action to add/modify any data in the application? It
will get processed but the resultant view will be denied to the user; because the flaw lies in just having
a view based access control in the applications. I am sure you will agree that a lot of processing for a
request is done before the “view” comes into picture in any design. So the request to process any
action will get processed successfully without authorization.
Consider a MVC based given in the figure below. Observe in the figure that the authentication check
is present only in the view pages.

OWASP CODE REVIEW GUIDE - V2.0

39

Observe that neither the controller servlet (central processing entity) nor the action classes have any
access control checks. So here, if the user requests for an internal action like add user details, etc.
without authentication it will get processed, but the only difference is that the user will be shown an
error page as resultant view will be disallowed to the user.

OWASP CODE REVIEW GUIDE - V2.0

40

Insecure POST-BACK’s in ASP.NET A similar flaw is predominantly observed in ASP.NET
applications where the developers tend to mix the code for handling POSTBACK’s and authentication
checks. Usually it is observed that the authentication check in the ASP.NET pages are not applied for
POSTBACKs, as indicated below. Here, if an attacker tries to access the page without authentication
an error page will be rendered. Instead, if the attacker tries to send an internal POSTBACK request
directly without authentication it would succeed.

Secure Design Recommendation:
It is imperative to place all validation checks before processing any business logic and in case of
ASP.NET applications independent of the POSTBACKs.

Review criteria
Check if the placement of the security checks is correct. The security controls like authentication
check must be place before processing any request.

Execution Flow

OWASP CODE REVIEW GUIDE - V2.0

41

Execution flow is another important consideration of design. The execution flow must terminate
appropriately in case of an error condition. However, due to mishandling of some programming
entities there could be a big hole in the application, which would allow unrestricted access to
applications. One such flaw is related to – “sendRedirect” method in J2EE applications.

response.sendRedirect(“home.html”);

This method is used to send a redirection response to the user who then gets redirected to the
desired web component who’s URL is passesd an argument to the method. One such misconception
is that execution flow in the Servlet/JSP page that is redirecting the user stops after a call to this
method. Take a look at the code snippet below, it checks for authenticated session using an “if”
condition. If the condition fails the response.sendRedirect() is used to redirect the user to an error
page.

Note that there is code present after the If condition, which continues to fetch request parameters and
processes business logic for instance adding a new branch entry of a bank in this case.

What is the flaw?

OWASP CODE REVIEW GUIDE - V2.0

42

This flaw manifests as a result of the misconception that the execution flow in the JSP/Servlet page
stops after the “sendRedirect” call. However it does not; in this case the execution of the servlet would
continue even if an invalid session is detected by the “if” condition and thus the business logic will get
processed for unauthenticated requests.

Note: The fact that execution of a servlet or JSP continues even after sendRedirect() method, also
applies to Forward method of the RequestDispatcher Class. However,  tag is an
exception, it is observed that the execution flow stops after the use of  tag.

Secure Design Recommendation:
Since this flaw results from the assumption made by developers that control flow execution terminates
after a sendRedirect call, the recommendation would be to terminate the flow using a “return”
statement.

Review criteria
Check if there is an appropriate logic to terminate the execution flow is present in case of an error
condition. Check for similar instances of insecure security controls built using “sendRedirect” method.

References:
http://artechtalks.blogspot.in/
http://packetstormsecurity.com/files/119129/Insecure-Authentication-Control-In-J2EE.html

2.2.1.6 A Risk based approach to code review (Needs Content)
Lorem Ipsum

2.2.2 Crawling Code
Crawling code is the practice of scanning a code base of the review target in question. It is, in effect,
looking for key pointers wherein possible security vulnerability might reside. Certain APIs are related
to interfacing to the external world or file IO or user management, which are key areas for an attacker
to focus on. In crawling code we look for APIs relating to these areas. We also need to look for
business logic areas, which may cause security issues, but generally these are bespoke methods

OWASP CODE REVIEW GUIDE - V2.0

43

which have bespoke names and can not be detected directly, even though we may touch on certain
methods due to their relationship with a certain key API.
We also need to look for common issues relating to a specific language; issues that may not be
*security* related but which may affect the stability/availability of the application in the case of
extraordinary circumstances. Other issues when performing a code review are areas such a simple
copyright notice in order to protect one’s intellectual property. Generally these issues should be part of
your companies Coding Guidelines, and should be enforceable during a code review (i.e. a reviewer
can fail code review because the code violates something in the Coding Guidelines, regardless of
whether or not the code would work in its current state, and regardless on whether the original
developer agrees or not).
Crawling code can be done manually or in an automated fashion using automated tools. Crawling
code manually is probably not effective, as (as can be seen below) there are plenty of indicators,
which can apply to a language. Tools as simple as grep or wingrep can be used. Other tools are
available which would search for key words relating to a specific programming language. If you are
using a particular review tool, which allows you to specify strings to be highlighted in a review (e.g.
Python based review tools using pygments syntax highlighter, or an in-house tool for which you can
change the source code) then you could add the relevant string indicators from the lists below and
have them highlighted to reviewers automatically.

The following sections shall cover the function of crawling code for Java/J2EE, .NET and Classic ASP.
This section is best used in conjunction with the transactional analysis section also detailed in this
guide.
- Searching for Key Indicators
The basis of the code review is to locate and analyse areas of code that may have application security
implications. Assuming the code reviewer has a thorough understanding of the code, what it is
intended to do, and the context in which it is to be used, firstly one needs to sweep the code base for
areas of interest.
This can be done by performing a text search on the code base looking for keywords relating to APIs
and functions. Below is a guide for .NET framework 1.1 & 2.0
2.2.2.1 Searching for Code in .NET
Firstly one needs to be familiar with the tools one can use in order to perform text searching, following
this one needs to know what to look for.
In this section we will assume you have a copy of Visual Studio (VS) .NET at hand. VS has two types
of search "Find in Files" and a cmd line tool called Findstr.

OWASP CODE REVIEW GUIDE - V2.0

44

To start off, one could scan thorough the code looking for common patterns or keywords such as
"User", "Password", "Pswd", "Key", "Http", etc... This can be done using the "Find in Files" tool in VS
or using findstring as follows:
findstr /s /m /i /d:c:\projects\codebase\sec "http" *.*
- HTTP Request Strings
Requests from external sources are obviously a key area of a security code review. We need to
ensure that all HTTP requests received are data validated for composition, max and min length, and if
the data falls with the realms of the parameter white-list. Bottom-line is this is a key area to look at and
ensure security is enabled.

request.ac

request.brows

request.fil

request.heade

request.httpmetho

cepttypes

er

es

rs

d

request.qu

request.form

request.co

request.certifi

request.rawurl

okies

cate

erystring

request.item

request.server
variables

request.ur

request.urlrefe

request.us

request.userla

request.IsSecureC

request.Total

l

rrer

eragent

nguages

onnection

Bytes

request.Bi

InputStream

HiddenFie

TextBox.Text

recordSet

naryRead

ld.Value

- HTML Output
Here we are looking for responses to the client. Responses which go unvalidated or which echo
external input without data validation are key areas to examine. Many client side attacks result from
poor response validation. XSS relies on this somewhat.
response.write

<% =

OWASP CODE REVIEW GUIDE - V2.0

HttpUtility

HtmlEncode

UrlEncode

45

innerText

innerHTML

- SQL & Database
Locating where a database may be involved in the code is an important aspect of the code review.
Looking at the database code will help determine if the application is vulnerable to SQL injection. One
aspect of this is to verify that the code uses either SqlParameter, OleDbParameter, or
OdbcParameter(System.Data.SqlClient). These are typed and treat parameters as the literal value
and not executable code in the database.


exec sp_

select from

insert

update

delete from where

delete

execute sp_

exec xp_

exec @

execute

executestatement

executeSQL

adodb

sqloledb

sql server

.Open

ADODB.recordset

New

@

setfilter

executeQuery

GetQuery
ResultIn
XML

driver

Server.CreateObject

.Provider

OleDbConnection

ExecuteReader

GetString

DataSource

SqlDataAdapter

SqlCom

Microsoft

mand

.Jet

Comman

StoredPro

dType

cedure

SqlDataReader

ExecuteReader

System.Data.sql

- Cookies

OWASP CODE REVIEW GUIDE - V2.0

46

Cookie manipulation can be key to various application security exploits, such as session
hijacking/fixation and parameter manipulation. One should examine any code relating to cookie
functionality, as this would have a bearing on session security.
System.Net.Cookie

HTTPOnly

document.cookie

- HTML Tags
Many of the HTML tags below can be used for client side attacks such as cross site scripting. It is
important to examine the context in which these tags are used and to examine any relevant data
validation associated with the display and use of such tags within a web application.

HtmlEncode

URLEncode