Secure Code Review 101

Secure Code Review 101Secure Code Review 101
Narudom Roongsiriwong, CISSPNarudom Roongsiriwong, CISSP
MiSSConf(SP2) Nov 19, 2016MiSSConf(SP2) Nov 19, 2016
Secure Code Review 101Secure Code Review 101
Narudom Roongsiriwong, CISSPNarudom Roongsiriwong, CISSP
MiSSConf(SP2) Nov 19, 2016MiSSConf(SP2) Nov 19, 2016

WhoAmI
● Lazy Blogger
– Japan, Security, FOSS, Politics, Christian
– http://narudomr.blogspot.com
● Information Security since 1995
● Web Application Development since 1998
● Head of IT Security and Solution Architecture, Kiatnakin
Bank PLC (KKP)
● Consultant for OWASP Thailand Chapter
● Committee Member of Cloud Security Alliance (CSA),
Thailand Chapter
● Consulting Team Member for National e-Payment project
● Contact: narudom.roongsiriwong@owasp.org

Security controls cannot
deal with broken business
logic such as A2, A4 and A7
Security controls cannot
deal with broken business
logic such as A2, A4 and A7
Software weaknesses
reduction down to zero is
possible
Software weaknesses
reduction down to zero is
possible
Reduce Security Weaknesses vs
Increase Security Controls

What is Secure Code Review?
● Aim to identify security flaws in the application
related to its features and design, along with the
exact root causes.
● Verify that the proper security and logical controls
are present, work as intended, and have been
invoked in the right places.
● Assure application developers are following secure
development techniques.
● Combine human effort and advanced static analysis
tools.

Code Review and Secure Code Review
● Secure Code Review enhances the standard Code
Review practice with security considerations.
– Security standards
– Security risks in the code base
– Security context
● Reviewers must have the necessary skills and secure
coding knowledge to effectively evaluate the code.

Code Review in Secure SDLC
Code Review

How Code Review Reduces Costs on Bug Fixes
Relative cost of security fixes, based on time of detection
Source: The National Institute of Standards and Technology (NIST)
Code Review
Penetration
Testing

Method Comparison to Cover High Level Topics
Good
Some
None
Source: Code Review Guide 2.0 (Alpha Release)

Method Comparison Against OWASP Top 10 Risks
Good
Some
None
Source: Code Review Guide 2.0 (Alpha Release)

Factors to Consider in Code Review Process
● Risks
● Purpose & Context
– A payment web application will have higher security
standards than a promotional website.
● Lines of Code: the more lines, the more bugs
● Programming Language
– Unmanaged code → Chances of buffer overflow
– PHP → Remote code execution
● Resources, Time & Deadlines

Code Review Preparation
● Application Features and Business Rules
– BR, SRS, etc
● Context
● Sensitive Data
● User Roles and Access Rights
● Application Type
– Web, Desktop, Web Service, Mobile, Hybrid
● Language and Its Security Features and Issues
● Design / Framework
– MVC, Strut, Spring, Hibernate, YII, CakePHP
● Company Standards and Guidelines

Code Review Checklist
● Data Validation
● Authentication
● Session Management
● Authorization
● Cryptography
● Error Handling
● Logging
● Security Configuration
● Network Architecture

Advantage & Disadvantage of Source Code Scanner
● Advantage
– Reduction in manual
efforts
– Find all the instances of
the vulnerabilities
– Source to sink analysis
– Elaborate reporting
format
● Disadvantage
– Business logic flaws
remain untouched
– Limited scope
– Design flaws
– False positives

Name
Programming Language Support
OS
Java PHP .NET C C++ Python Other
CodePro X W L M
FindBugs X W L
FxCop X W
Flawfinder X X L
Milk X X W L
MOPS X L
OWASP Code Crawler X X W
OWASP ORIZON X X X L
OWASP O2 Platform X X Javascript W
OWASP LAPSE X W L M
PMD X X X Javascript, XML,
XSL W L
PREfast X X W L
RIPS-Scanner X
SonarQube X X X X Delphi, Javascript,
XML W L M
Sprint X W L
StypeCop X W
Yasca X X X X X HTML, , Javascript,
Cobol, Coldfusion W L
Free Source Code Scanners

Example: SonarQube with OWASP Plugin

Let’s Go Back to Basic without Code Scanner
● Use your favorite text editor or IDE.
● “Find in Files” feature with RegEx is recommended.
● In this presentation will show you “Geany”, cross
platform text editor. https://www.geany.org

Review SQL Injection
● Cause of SQL injection vulnerability is from an SQL
command that constructs from the untrusted input.
● Common actions to interact with data are Create
(INSERT), Read (SELECT), Update, Delete.
● SELECT/UPDATE/DELETE are usually filtered only
some records, using WHERE.
● Some bad code use dynamic fields or tables, it’s also
able to be injected.
Sting custQuery =
“SELECT custName, address1, address2,
city, postalCode WHERE custID= ‘“ +
request.GetParameter(“id”) + “’“
Code
Data

Example: Find in Files for INSERT or WHERE

Example: Find in Files for INSERT or WHERE
1) Suspect
2) Is this an input
parameter?
3) Vulnerable from calling?

Review Remote Code Injection
● Both client side (JavaScript) and
server side (ex. PHP) scripting
● Search for data from untrusted
sources could be inputs of
– eval (most of scripting language)
– include, require (PHP)
eva1

Review Hard-Coded Password/Encryption Key
● Hard-coded passwords or
key may compromise
system security in a way
that cannot be easily
remedied.
● Developers may create a
backdoor with hard-coded
username and password
for special credential.
● Forms of password for
databases and application
are likely to be “password”,
“pass”, “passwd” or “pwd”.
→ RegEx: pass|pwd
● Borland Interbase 4.0, 5.0, 6.0 was
discovered a special credentials,
username “politically” and
password “correct”, were inserted
into the credential table at
program startup.
dpb = dpb_string;
*dpb++ = gds__dpb_version1;
*dpb++ = gds__dpb_user_name;
*dpb++ = strlen (LOCKSMITH_USER);
q = LOCKSMITH_USER;
while (*q) *dpb++ = *q++;
*dpb++ = gds__dpb_password_enc;
strcpy (password_enc, (char
*)ENC_crypt(LOCKSMITH_PASSWORD,
PASSWORD_SALT));
q = password_enc + 2;
*dpb++ = strlen (q);
while (*q) *dpb++ = *q++;
dpb_length = dpb – dpb_string;
isc_attach_database (status_vector, 0,
GDS_VAL(name), &DB, dpb_length,
dpb_string);

Review Poor Logging Practices
● Use of a System Output Stream
– Using System.out or System.err rather than a dedicated logging.
– Log messages may accidentally be returned to the end users and
expose sensitive information
public class MyClass
public void debug(Object message) {
System.out.println(message);
}
}
● Logger Not Declared Static Final
– Loggers should be declared to be static and final.
– Use the same logger for the duration of the program.
– The following statement errantly declares a non-static logger.
private final Logger logger = Logger.getLogger(MyClass.class);

Review Session Management
● .Net ASPX web.config
<authentication mode=”Forms”>
<forms loginUrl=”member_login.aspx”
cookieless=”UseCookies”
requireSSL=”true”
path=”/MyApplication” />
</authentication>
● Java web.xml
<session-config>
<cookie-config>
<secure>true</secure>
</cookie-config>
</session-config>
● PHP.ini
session.cookie_lifetime=0
session.use_cookies=On
session.use_only_cookies=On
session.use_strict_mode=On
session.cookie_httponly=On
session.cookie_secure=On
session.gc_maxlifetime=[choose smallest possible]
session.use_trans_sid=Off

Conclusion
● Code scanner absolutely helps code reviewers but
they are lacks of capabilities and usually create false
positive.
● Code reviewers should know the specific language
and framework of codes they reviews
● The justification must rely on the context and
requirements of the application together with
standards and guidelines

Secure Code Review 101

More Related Content

What's hot

Similar to Secure Code Review 101

More from Narudom Roongsiriwong, CISSP

Recently uploaded

Secure Code Review 101