10 Secure Code Review

“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 non-trivial application has had security vulnerabilities. If code has
not been reviewed for security holes, the likelihood that the application has problems is virtually 100%.

Still, there are many organizations that choose not to know about the security of their code. To them, consider
Rumsfeld’s cryptic explanation of what we actually know:
“...we know, there are known knowns; there are things we know we know. We also know there are known un-
knowns; 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

If informed decisions are being made based on a measurement of risk in the enterprise, which will be fully
supported. However, if risks are not being understood, the company is not being duly diligent, and is being
irresponsible both to shareholders and customers.

5.2 What is Secure Code Review?

Code review aims to identify security flaws in the application related to its features and design, along with the
exact root causes. With the increasing complexity of applications and the advent of new technologies, the
traditional way of testing may fail to detect all the security flaws present in the applications. One must under-
stand the code of the application, external components, and configurations to have a better chance of finding
the flaws. Such a deep dive into the application code also helps in determining exact mitigation techniques
that can be used to avert the security flaws.

It is the process of auditing the source code of 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 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.

Secure code review allows a company to assure 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. At the 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 spec-
trum is an inexperienced person with a text editor. At the other end of the scale is an expert security team with
advanced static analysis (SAST) tools. Unfortunately, it takes a fairly serious level of expertise to use the current
application security tools effectively. They also don’t understand dynamic data 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. They 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 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, which automated tools, simply cannot check.
 Secure Code Review 11

5.3 What is the difference between Code Review and Secure Code Review?
The Capability Maturity Model (CMM) is a widely recognized process model for measuring the development
processes of a software development organization. It ranges from ‘level 1’ where development processes are
ad hoc, unstable and not repeatable, to ‘level 5’ where the development processes are well organized, docu-
mented and continuously improving. It is assumed that a company’s development processes would start out
at level 1 when starting out (a.k.a start-up mode) and will become more defined, repeatable and generally
professional as the organization matures and improves. Introducing the ability to perform code reviews (note
this is not dealing with secure code review yet) comes in when an organization has reached level 2 (Repeat-
able) or level 3 (Defined).

Secure Code Review is an enhancement to the standard code review practice where the structure of the re-
view process places security considerations, such as company security standards, at the forefront of the de-
cision-making. Many of these decisions will be explained in this document and attempt to ensure that the
review process can adequately cover security risks in the code base, for example ensuring high risk code is
reviewed in more depth, ensuring reviewers have the correct security context when reviewing the code, en-
suring reviewers have the necessary skills and secure coding knowledge to effectively evaluate the code.

5.4 Determining the Scale of a Secure Source Code Review?

The level of secure source code review will vary depending on the business or regulatory needs of the software, the
size of the software development organization writing the applications and the skills of the personnel. Similar to
other aspects of software development such as performance, scalability and maintainability, security is a measure of
maturity in an application. Security is one of the non-functional requirements that should be built into every serious
application or tool that is used for commercial or governmental purposes.

If the development environment consists of one person programming as a hobby and writing a program to track
their weekly shopping in visual basic (CMM level 1), it is unlikely that that programmer will use all of the advice
within this document to perform extensive levels of secure code review. On the other extreme, a large organization
with thousands of developers writing hundreds of applications will (if they wish to be successful) take security very
seriously, just like they would take performance and scalability seriously.

Not every development organization has the necessity, or resources, to follow and implement all of the topics in
this document, but all organizations should be able to begin to write their development processes in a way that
can accommodate the processes and technical advice most important to them. Those processes should then
be extensible to accommodate more of the secure code review considerations as the organization develops and
matures.

In a start-up consisting of 3 people in a darkened room, there will not be a ‘code review team’ to send the code to,
instead it’ll be the bloke in the corner who read a secure coding book once and now uses it to prop up his monitor.

In a medium sized company there might be 400 developers, some with security as an interest or specialty, however
the organizations processes might give the same amount of time to review a 3 line CSS change as it gives to a
redesign of the flagship products authentication code. Here the challenge is to increase the workforce’s secure
coding knowledge (in general) and improve the processes through things like threat modelling and secure code
review.

For some larger companies with many thousands of developers, the need for security in the S-SDLC is at its greatest,
but process efficiency has an impact on the bottom line. Take an example of a large company with 5,000 developers.
If a change is introduced to the process that results in each developer taking an extra 15 minutes a week to perform
a task, suddenly that’s 1,250 hours extra each week for the company as a whole This results in a need for an extra 30
full time developers each year just to stay on track (assuming a 40 hour week). The challenge here is to ensure the
security changes to the lifecycle are efficient and do not impede the developers from performing their task.
12 Secure Code Review

Skilling a Workforce for Secure Code Review

There seems to be a catch-22 with the following sentiment; as many code developers are not aware or skilled in
security, a company should implement peer secure code reviews amongst developers.
How does a workforce introduce the security skills to implement a secure code review methodology? Many
security maturity models (e.g. BSIMM or OpenSAMM) discuss the concept of a core security team, who are skilled
developers and skill security subject matter experts (SMEs). In the early days of a company rolling out a se-
cure code review process, the security SMEs will be central in the higher risk reviews, using their experience and
knowledge to point out aspects of the code that could introduce risk.

As well as the core security team a further group of developers with an interest in security can act as team local
security SMEs, taking part in many secure code reviews. These satellites (as BSIMM calls them) will be guided by
the core security team on technical issues, and will help encourage secure coding.

Over time, an organization builds security knowledge within its core, and satellite teams, which in turns spreads
the security knowledge across all developers since most code reviews will have a security SME taking part.

The ‘on-the-job’ training this gives to all developers is very important. Whereas an organization can send their de-
velopers on training courses (classroom or CBT) which will introduce them to common security topics and create
awareness, no training course can be 100% relevant to a developer’s job. In the secure code review process, each
developer who submits their code will receive security related feedback that is entirely relevant to them, since
the review is of the code they produced.

It must be remembered though, no matter what size the organization, the reason to perform secure code re-
view is to catch more bugs and catch them earlier in the S-SDLC. It is quicker to conduct a secure code review
and find bugs that way, compared to finding the bugs in testing or in production. For the 5,000-person orga-
nization, how long will it take to find a bug in testing, investigate, re-code, re-review, re-release and re-test?
What if the code goes to production where project management and support will get involved in tracking the
issue and communicating with customers? Maybe 15 minutes a week will seem like a bargain.

5.5 We Can’t Hack Ourselves Secure

Penetration testing is generally a black-box point in time test and should be repeated on each release (or
build) of the source code to find any regressions. Many continuous integration tools (e.g. Jenkins/Hudson)
allow repeatable tests, including automated penetration tests, to be run against a built and installed version
of a product.

As source code changes, the value of the findings of an unmaintained penetration tests degrade with time.
There are also privacy, compliance and stability and availability concerns, which may not covered by penetra-
tion testing, but can be covered in code reviews. Data information leakage in a cloud environment for example
may not be discovered, or allowed, via a penetration test. Therefore penetration testing should be seen as an
important tool in the arsenal, but alone it will not ensure product software is secure.

The common methods of identifying vulnerabilities in a software project are:

• Source Code Scanning using automated tools that run against a source code repository or module, finding
string patterns deemed to potentially cause security vulnerabilities.
 Secure Code Review 13

• Automated Penetration Testing (black/grey box) through penetrating testing tools automatic scans, where
the tool is installed on the network with the web site being tested, and runs a set of pre-defined tests against
the web site URLs.

• Manual Penetration Testing, again using tools, but with the expertise of a penetration tester performing more
complicated tests.

• Secure Code Review with a security subject matter expert.

It should be noted that no one method will be able to identify all vulnerabilities that a software project might
encounter, however a defense-in-depth approach will reduce the risk of unknown issues being including in
production software.

During a survey at AppSec USA 2015 the respondents rated which security method was the most effective in
finding:

1) General security vulnerabilities

2) Privacy issues
3) Business logic bugs
4) Compliance issues (such as HIPPA, PCI, etc.)
5) Availability issues

The results are shown in figure 1.

Figure 1: Survey relating detection methods to general vulnerability types

35
30
25
20
15
10
5
0
Vulnerabilities Privacy Business Logic Compliance Availability
(HIPPA)

Source Code Scanning Tools

Automated Scan

Manual Pen Test

Manual Code Review
14 Secure Code Review

Figure 2: Survey relating detection methods to OWASP Top 10 vulnerability types

35
30
25
20
15
10
5
0
A1 A2 A3 A4 A5 A6 A7 A8 A9 A10

Source Code Scanning Tool

Automated Scan

Manual Pen Test

Manual Code Review

These surveys show that manual code review should be a component of a company’s secure lifecycle, as in
many cases it is as good, or better, than other methods of detecting security issues.

5.6 Coupling Source Code Review and Penetration Testing

The term “360 review” refers to an approach in which the results of a source code review are used to plan and
execute a penetration test, and the results of the penetration test are, in turn, used to inform additional source
code review.

Figure 3: Code Review and Penetration Testing Interactions

CODE
REVIEW

EXPLOITED SUSPECTED KNOWN

VULNERABILITIES VULNERABILITIES

PENETRATION
TEST
 Secure Code Review 15

Knowing the internal code structure from the code review, and using that knowledge to form test cases and
abuse cases is known as white box testing (also called clear box and glass box testing). This approach can lead to
a more productive penetration test, since testing can be focused on suspected or even known vulnerabilities. Us-
ing knowledge of the specific frameworks, libraries and languages used in the web application, the penetration
test can concentrate on weaknesses known to exist in those frameworks, libraries and languages.

A white box penetration test can also be used to establish the actual risk posed by a vulnerability discovered
through code review. A vulnerability found during code review may turn out not to be exploitable during pen-
etration test due to the code reviewer(s) not considering a protective measure (input validation, for instance).
While the vulnerability in this case is real, the actual risk may be lower due to the lack of exposure. However there
is still an advantage to adding the penetration test encase the protective measure is changed in the future and
therefore exposes the vulnerability.

While vulnerabilities exploited during a white box penetration test (based on secure code review) are certainly
real, the actual risk of these vulnerabilities should be carefully analyzed. It is unrealistic that an attacker would
be given access to the target web application’s source code and advice from its developers. Thus, the risk that
an outside attacker could exploit the vulnerabilities found by the white box penetration tester is probably low-
er. However, if the web application organization is concerned with the risk of attackers with inside knowledge
(former employees or collusion with current employees or contractors), the real-world risk may be just as high.

The results of the penetration test can then be used to target additional areas for code review. Besides address-
ing the par-ticular vulnerability exploited in the test, it is a good practice to look for additional places where that
same class of vulnerability is present, even if not explicitly exploited in test. For instance, if output encoding is
not used in one area of the application and the penetration test exploited that, it is quite possible that output
encoding is also not used elsewhere in the application.

5.7 Implicit Advantages of Code Review to Development Practices

Integrating code review into a company’s development processes can have many benefits which will depend
upon the processes and tools used to perform code reviews, how well that data is backed up, and how 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 are long gone, today many tools exist to make code review more efficient
and to track the review records and decisions. When the code review process is structured correctly, the act of
reviewing code can be efficient and provide educational, auditable and historical benefits to any organization.
This section provides a list of benefits that a code review procedure can add to development organization.

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 old code. Various schools of opinion exist on code documentation, both within the code (comments) and
external to the code (design and functional documents, wikis, etc.). Opinions range from zero-documentation
tolerance through to near-NASA level documentation, where the size of the documentation far exceeds the size
of the code module.

Many of the discussions that occur during a code review, if recorded, would provide valuable information (con-
text) 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 restruc-
tured, or an algorithm changed, there is a development story unfolding in front of the reviewers eyes which can
be used by future coders on the module, who are not involved in the review meetings.
16 Secure Code Review

Capturing those review discussions in a review tool automatically and storing them for future reference will pro-
vide the development organization with a history of the changes on the module which can be queried at a lat-
er time by new developers. These discussions can also contain links to any architectural/functional/design/test
specifications, bug or enhancement numbers.

Verification that the change has been tested

When a developer is about to submit code into the repository, how does the company know they have sufficient-
ly 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 review the tests
and suggest further testing that may have been missed by the author.

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

Coding education for junior developers

After an employee learns the basics of a language and read a few of the best practices book, how can they 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 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 its code will impact the 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 siloed teams. Here the code review author can reach out to
other teams to gain their insight, and allow those other teams to review their modules, and everyone then learns
a bit more about the company’s code base.

Pre-warning of integration clashes

In a busy code base there will be times (especially on core code modules) where multiple developers can write
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.

Secure Coding Guidelines Touch Point

Many development environments have coding guidelines which new code must adhere to. Coding guidelines
can take many forms. It’s worth pointing out that security guidelines can be a particularly relevant touch point
 Secure Code Review 17

within a code review, as unfortunately the secure coding issues are understood only by a subset of the develop-
ment 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 guidelines could be enforced.

5.8 Technical Aspects of Secure Code Review

Security code reviews are very specific to the application being reviewed. They may highlight some flaws that
are new or specific to the code implementation of the application, like insecure termination of execution flow,
synchronization errors, etc. These flaws can only be uncovered when we understand the application code flow
and its logic. 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.

The application being reviewed might have been designed with some security controls in place, for example a
centralized blacklist, input validation, etc. These security controls must be studied carefully to identify if they
are fool-proof. According to the implementation of the control, the nature of attack or any specific attack vec-
tor 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.

There are various reasons why security flaws manifest in the application, like a lack of input validation or
parameter mishandling. In the process of a code review the exact root cause of flaws are exposed and the
complete data flow is traced. The term ‘source to sink analysis’ means to determine all possible inputs to the
application (source) and how they are being processed by it (sink). A sink could be an insecure code pattern
like a dynamic SQL query, a log writer, or a response to a client device.

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 (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 helps in understanding, which vulnerable inputs can lead to a
possibility of an exploit in the application.

Once a flaw is identified, the reviewer must enumerate all the possible instances present in the application.
This would not be a code review initiated by a code change, this would be a code scan initiated by manage-
ment based on a flaw being discovered and resources being committed to find if that flaw exists in other
parts of the product. For example, an application can be vulnerable to XSS vulnerability because of use of
un-validated inputs in insecure display methods like scriptlets ‘response.write’ method, etc. in several places.

5.9 Code Reviews and Regulatory Compliance

Many organizations with responsibility for safeguarding the integrity, confidentiality and availability of their
software and data need to meet regulatory compliance. This compliance is usually mandatory rather than a
voluntary step taken by the organization.

Compliance regulations include:

• PCI (Payment Card Industry) standards
• Central bank regulations
18 Secure Code Review

• Auditing objectives
• HIPPA

Compliance is an integral part of software security development life-cycle and code review is an important
part of compliance as many rules insist on the execution of code reviews in order to comply with certain reg-
ulations.

To execute proper code reviews that meet compliance rules it is imperative to use an approved methodolo-
gy. Compliance requirements such as PCI, specifically requirement 6: “Develop and maintain secure systems”,
while PCI-DSS 3.0, which has been available since November 2013, exposes a series of requirements which
apply to development of software and identifying vulnerabilities in code. The Payment Card Industry Data
Security Standard (PCI-DSS) became a mandatory compliance step for companies processing credit card pay-
ments in June 2005. Performing code reviews on custom code has been a requirement since the first version
of the standard.

The PCI standard contains several points relating to secure application development, but this guide will focus
solely on the points, which mandate code reviews. All of the points relating to code reviews can be found in
requirement 6 “Develop and maintain secure systems and applications”.

5.10 PCI-DSS Requirements Related to Code Review

Specifically, requirement 6.3.2 mandates a code review of custom code. Reviewing custom code prior to re-
lease to production or customers in order to identify any potential coding vulnerability (using either manual
or automated processes) to include at least the following:

• Code changes are reviewed by individuals other than the originating code author, and by individuals knowl-
edgeable about code review techniques and secure coding practices.

• Code reviews ensure code is developed according to secure coding guidelines

• Appropriate corrections are implemented prior to release.

• Code review results are reviewed and approved by management prior to release.

Requirement 6.5 address common coding vulnerabilities in software-development processes as follows:

• Train developers in secure coding techniques, including how to avoid common coding vulnerabilities, and
understanding how sensitive data is handled in memory.

• Develop applications based on secure coding guidelines.

The PCI Council expanded option one to include internal resources performing code reviews. This added
weight to an internal code review and should provide an additional reason to ensure this process is performed
correctly.

The Payment Application Data Security Standard (PA-DSS) is a set of rules and requirements similar to PCI-DSS.
However, PA-DSS applies especially to software vendors and others who develop payment applications that
store, process, or transmit cardholder data as part of authorization or settlement, where these payment appli-
cations are sold, distributed, or licensed to third parties.
 Secure Code Review 19

PA-DSS Requirements Related to Code Review

Requirements regarding code review are also applied since these are derived from PA-DSS in requirement 5
(PCI, 2010):

5.2 Develop all payment applications (internal and external, and including web administrative access to prod-
uct) based on secure coding guidelines.

5.1.4 Review of payment application code prior to release to customers after any significant change, to identi-
fy any potential coding vulnerability.

Note: This requirement for code reviews applies to all payment application components (both internal and
public-facing web applications), as part of the system development life cycle. Code reviews can be conducted
by knowledgeable internal personnel or third parties.