PA193 - Secure coding
principles and practices
Security Code Review
Petr Švenda svenda@fi.muni.cz
PROBLEM
2 | PA193 - Security Code Review
Example problem – Debian RNG flaw
• Linus’s law
– “Given enough eyeballs, all bugs are shallow”
– https://en.wikipedia.org/wiki/Linus%27_Law
• Flaw in Debian’s random number generator (2008)
– CVE-2008-0166
– http://www.debian.org/security/2008/dsa-1571
– lead to predictable random numbers
– improper change to OpenSSL random generator
– persisted for almost two years!
– lead to only 262148 possible openSSH keys
• Change made based on static and dynamic analysis tools
recommendation!
3 | PA193 - Security Code Review
Debian RNG flaw
• Valgrind and IBM’s Purify reports problems
– usage of uninitialized variable
– OpenSSL crypto/rand/md_rand.c
• Discussion of maintainers (before and after change)
– http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=363516
4 | PA193 - Security Code Review
MD_Update(&m,buf,j);
MD_Update(&m,buf,j); /* purify complains */
Fatal mistake
5 | PA193 - Security Code Review
Morale
1. Access to source code doesn’t guarantee bugfree
code
2. Usage of automated tools can provide great
advantage, but deep understanding of code
before change must remain
3. Code review eventually spotted the problem
6 | PA193 - Security Code Review
SECURITY CODE REVIEW
7 | PA193 - Security Code Review
Resources
• Review process and techniques are extensively
based on the excellent book “The Art of Software
Security Assessment: Identifying and Preventing
Software Vulnerabilities” by Mark Dowd,
John McDonald, Justin Schuh
• (Book is available in faculty library)
8 | PA193 - Security Code Review
| PA193 - Security Code Review
Security code review
• Architecture overview
– Design choices and possible design flaws
• Code review
– How well is architecture actually implemented
• Whitebox, greybox & blackbox testing
– different level of access to code and documentation
• Available tools
– mainly for code review
9
Different targets
• Review of selected cryptographic functions and modes
– E.g., AES in CTR
• Review of custom implementation of protocols
– E.g., custom payment protocol
• Review of security-related code
– E.g., policy enforcement, access control…
• Review of non-security code with impact on security
– E.g., buffer overflow or XSS anywhere
10 | PA193 - Security Code Review
Application review phases
1. Pre-assessment
2. Application review
3. Documentation and analysis
4. Remediation support
11 | PA193 - Security Code Review
Timeline
• Good reviewer
– ranges between 100 to 1000 lines of code an hour
– highly dependent also on code complexity
– flexibility must be allowed
• Keep track of your previous progress
– and get feeling for your speed
– helps you making better future estimations
12 | PA193 - Security Code Review
Information Collection
• Developer interviews
• Developer documentation
• Standards documentation
• Source profiling
• System profiling
• Significantly cheaper to ask developer/designer
then figure yourself
– But don’t trust developers too much 
13 | PA193 - Security Code Review
Common problems
• Design documentation not available at all
• Design documentation is outdated
• Third party components without documentation
• Developers not available or not cooperating
• Limited time for everything
14 | PA193 - Security Code Review
Iterative process
1. Plan your next work
2. Perform auditing strategy you selected
– and make extensive notes
3. Reflect on time spend
– what you have learned
4. Repeat from step 1.
15 | PA193 - Security Code Review
Top-down approach
• Top-down approach
– water-fall like approach
1. start from design specification
2. establish threat model
3. find design vulnerabilities first
4. find logical implementation vulnerabilities second
5. find low-level implementation bugs third
• Good results if design documentation is accurate
– but that is usually not the case
– something is missing or implemented differently
16 | PA193 - Security Code Review
Bottom-up approach
• Bottom-up approach
– starts with implementation
– targets low-level implementation vulnerabilities first
• e.g., by automated tools
– higher-level threat and design documentation later
• when understanding of application is much better
• Works well even if design documentation is not accurate
– but is slow as you need to read a lot of code that is NOT security
relevant
• Necessity for maintaining design model continuously
– e.g., DFD sketches and class diagrams
17 | PA193 - Security Code Review
Hybrid approach
• Combination of top-down and bottom-up approaches
• Focus on high-level characteristics
– General application purpose
– Assets and entry points
– Components and modules
– Inter-module relations
– Fundamental security
– Major trust boundaries
18 | PA193 - Security Code Review
Verify the progress you are making
• Ask often following questions:
• What have you learned about the application?
• Are you focusing on the most security-relevant
components?
• Have you gotten stuck on real problem or gone
down some rabbit hole?
19 | PA193 - Security Code Review
The Art of Software Security Assessment: Identifying and Preventing Software Vulnerabilities
Verify the progress you are making (cont.)
• Ask often following questions:
• Does your master ideas list have many plausible
entries?
• Have you been taking adequate notes and
recorded enough detail for review purposes?
• If you're working from application models and
documentation, do these models reflect the
implementation accurately?
20 | PA193 - Security Code Review
The Art of Software Security Assessment: Identifying and Preventing Software Vulnerabilities
| PA193 - Security Code Review
Security code review - hints
• You will always have a limited time
– try to rapidly build overall picture
– use tools to find low hanging fruit (attacker will start with these)
• Focus on most sensitive and problematic areas
– use tools to focus your analysis scope
• More eyes can spot more problems
– experts on different areas
• It’s creative process
– be pragmatic, flexible, and results driven
• Have the rights skills
– you should know programming as well as have security mindset
21
Present results (Finding summary)
• Location of the vulnerability
• Vulnerability class
• Vulnerability description
• Prerequisites (for exploiting vulnerability)
• Business impact (on assets)
• Remediation (how to fix)
• Risk
• Severity
• Probability
22 | PA193 - Security Code Review
Finding summary - example
23 | PA193 - Security Code Review
Problem identification: DSA-1571-1 openssl
Severity: critical
Risk: high - directly exploitable by external attacker
Problem description: crypto/rand/md_rand.c:276 & 473 – The random number
generator in Debian's openssl package is predictable. This is caused by an
incorrect Debian-specific change to the openssl package. One of the sources of
a randomness based on usage of uninitialized buffer buff is removed.
Remediation: revert back to usage of uninitialized buffer buff
| PA193 - Security Code Review
Architecture review
| PA193 - Security Code Review
Architecture overview
• Get all information you can quickly
• Assets
– What has the value in the system?
– What damage is caused when successfully attacked?
– What mechanisms are used to protect assets?
• Roles
– Who has access to what?
– What credentials needs to be presented?
• Thread model
– What is expected to do harm?
– What are you defending against?
25
| PA193 - Security Code Review
Architecture review (2)
• Usage of well established techniques and
standards
• Comparison with existing schemes
– What is the advantage of new scheme?
– Why changes were made?
• Security tradeoffs documented
– Possible threat, but unmitigated?
– Is documented or overlooked?
26
| PA193 - Security Code Review
Sensitive data flow mapping
• Identify sensitive data
– password, key, protected data...
• Find all processing functions
– and focus on them
• Create data flow between functions
– e.g. Doxygen call graph
• Inspect when functions can be called
– Is key schedule validity checked?
– Can be function called without previous function calls?
• Where are sensitive data stored between calls?
27
| PA193 - Security Code Review
Protocol design (and implementation)
• Packet confidentiality, integrity and authenticity
• Packet removal/insertion detection
• Replay attack
• Reflection attack
• Man in the middle
28
| PA193 - Security Code Review
Cryptography usage
• CIA (Confidentiality, Integrity, Availability)
– Plaintext data over insecure channel? Encrypted only?
– Can be packet send twice (replay)?
– What is the application response on data modification?
• What algorithms are used
– Broken/insecure algorithms? MD5? simple DES?
• What key lengths are used?
– < 90 bits symmetric crypto?
– < 1024 bits asymmetric crypto?
• Random number generation
– Where the key comes from?
– Is source entropic enough? (remember Debian flow)
– srand() & rand()?
29
| PA193 - Security Code Review
Cryptography usage (2)
• Key creation
– Where the keys originate? Enough entropy?
– Who has access?
• Key storage
– Hard-coded keys
– Keys in files in plaintext
– Keys over insecure channels
– Keys protected by less secure keys
• Key destruction
– How are keys erased from memory?
– Can exception prevent key erase?
30
| PA193 - Security Code Review
Cryptography implementation
• Implementation from well known libraries?
• Own algorithms?
– security by obscurity?
– usually not secure enough
• Own modifications?
– Why?
– sometimes used to prevent compatible programs
– decreased number of rounds?
– Performance optimization with security impact?
31
CODE INSPECTION
32 | PA193 - Security Code Review
Example process
1. Start review by suite of static analysis tools
– approximately up to 40-50% of software bugs can be found
– but incapable of finding application flaws and business logic vulns.
2. Results used to create prioritized list for human review
– security mechanisms to review
– potential security vulnerabilities to investigate
3. Manual inspection of issues in prioritized list
– use and abuse cases
– various code inspection strategies
4. Threat modeling used for large codebases (>100k loc)
– inspect impact of generally high-risk threat on application
33 | PA193 - Security Code Review
Code navigation
• Control-flow sensitive navigation
– follow function calls
– e.g., what parts of program are reachable from set of
functions callable without previous authentication?
• Data-flow sensitive navigation
– follow flows of interesting data
– e.g., password from input to verification and storage
• Code navigation tools provide great help
– call graphs (Doxygen, Performance profilers)
– tainted values (e.g., taintgrind)
– ...
34 | PA193 - Security Code Review
Code auditing strategies
• Code comprehension (CC) strategies
– analysing the source code directly to discover
vulnerabilities
• Candidate point (CP) strategies
– create a list of potential issues (via some mechanism)
– examine the source code for relevance of these issues
• Design generalization (DG) strategies
– reviewing the implementation and inferring higher-level
design abstractions
– medium- to high-level logic and design flaws
35 | PA193 - Security Code Review
| PA193 - Security Code Review
Code comprehension (CC) strategies
CC strategy - Trace Malicious Input
• Start at entry point to the system
– e.g., user input
• Trace flow of code forward with data flow analysis
– functions processing user input
• Set of possible “bad” inputs is created
– e.g., escaped shell command
• Code is examined for potential security issue
– where is user input “executed”?
37 | PA193 - Security Code Review
Trace Malicious Input - characteristics
38 | PA193 - Security Code Review
The Art of Software Security Assessment: Identifying and Preventing Software Vulnerabilities
Slow 
Identify subtle or
abstract flaws 
Complex 
CC strategy - Analyse Module & Algorithm
• Reading the code line by line from the beginning
• Do not follow function calls
• Writing down potential issues spotted
• Algorithm analysis is similar to module analysis, but
module implementation is usually longer
• Effective, if the code is not too long, but mentally
exhausting
– overlooked problems after some time, time-demanding
39 | PA193 - Security Code Review
Analyse a Module - characteristics
40 | PA193 - Security Code Review
The Art of Software Security Assessment: Identifying and Preventing Software Vulnerabilities
Slow 
Identify subtle or
abstract flaws 
Mentally
difficult 
CC strategy – other useful strategies
• Analyse a Class or Object
– implementation of small unit
• Trace Black Box Hits
– focus on areas where fuzzers etc. found problems
– e.g., by debugging with value used to crash application
• Automated Source Analysis Tool
– used to generate candidate points
41 | PA193 - Security Code Review
| PA193 - Security Code Review
Candidate points (CP) strategies
Candidate points strategies
1. Use some tool or process for identifying candidate points
2. Deeper follow-up inspection by other (e.g., CC) strategy
• Simple Lexical Candidate Points
– patterns of common vulnerabilities (full text search, grep-like tool)
– deprecated functions (e.g., gets), strings like “key”, “password”...
– static analysis tools, e.g., Cppcheck rules
43 | PA193 - Security Code Review
Candidate points strategies
• Simple Binary Candidate Points
– generate candidate points from binary only (unavailable source code)
– list or search for specific strings in binary (e.g., password, AES
constants)
– search for interesting system calls (e.g., system())
– use disassembling, or binary debugging (e.g., IDA, OllyDbg)
• Application-Specific Candidate Points
– patterns of mistakes for particular application
– learned from previous code/binary analysis
– e.g., new custom rule for Cppcheck
44 | PA193 - Security Code Review
TOOLS
45 | PA193 - Security Code Review
Handy tools
• Syntax highlighting, full text search
– any reasonable editor
• Regular expression tools (grep)
– allow for more complex searches
• Automatic generation of call graphs
– Doxygen, Visual Studio and many other tools
– Performance profilers provides interesting information
46 | PA193 - Security Code Review
Handy tools
• Static and dynamic analyzers
– detect multiple issues (=> candidate points)
– annotations (e.g., SAL) will help even further
• Fuzzing tools
– behavior under stress, error messages...
• Mind-mapping software
– build and do not forget information you got
• Pen&Pencil
– still of great help (flexible)
47 | PA193 - Security Code Review
ANTI-PATTERNS
48 | PA193 - Security Code Review
| PA193 - Security Code Review
(Security) Antipatterns
• Common defective process and implementation
within organization
• Opposite to design patterns
– see http://sourcemaking.com/design_patterns
• Read http://sourcemaking.com/antipatterns
– good description, examples and how to solve
– not limited to object oriented programming!
49
Security anti-patterns
• Software development anti-patterns
– http://sourcemaking.com/antipatterns/software-
development-antipatterns
• Tesco password handling
– http://www.troyhunt.com/2012/07/lessons-in-website-
security-anti.html
• Critique of some usages of OAuth
– http://adactio.com/journal/1357/
50 | PA193 - Security Code Review
Recommended reading
• Process of security code review
– http://ieeexplore.ieee.org/stamp/stamp.jsp?arnumber=01668009
• Software Security Code Review
– http://www.softwaremag.com/l.cfm?doc=2005-07/2005-07code
• Performing security Review (Microsoft)
– http://silverstr.ufies.org/blog/msdn-webcast-code-review.pdf
• SDL security code review process (MS Security Push)
– http://msdn.microsoft.com/en-us/library/cc307418.aspx
• OWASP security review
– https://www.owasp.org/index.php/Security_Code_Review_in_the_SDLC
• On the effectiveness of code review
– http://www.cs.berkeley.edu/~daw/papers/coderev-essos13.pdf
51 | PA193 - Security Code Review
An Empirical Study on the Effectiveness
of Security Code Review
• Main findings (cited from
http://www.cs.berkeley.edu/~daw/papers/coderev-essos13.pdf):
1. None of the subjects found all confirmed
vulnerabilities
2. More experience does not necessarily mean that
the reviewer will be more accurate or effective,
3. Reports of false vulnerabilities were significantly
correlated with reports of valid vulnerabilities
52 | PA193 - Security Code Review
| PA193 - Security Code Review
Recommended reading
• Why cryptosystems fail, R. Anderson
– http://www.cl.cam.ac.uk/~rja14/Papers/wcf.pdf
• Static code analysis tools
– http://en.wikipedia.org/wiki/List_of_tools_for_static_code_analysis
• Security in web applications (OWASP)
– http://www.owasp.org/index.php/Code_Review_Introduction
53
CONCLUSIONS
54 | PA193 - Security Code Review
Conclusions
• Plan your work and time (work iteratively)
• Different reviews needs different techniques (be flexible)
• Code review is creative process (have fun)
• Tools can help you a lot (use them)
• but main part of work is up to you
• Code review also contains human interaction (be polite)
55 | PA193 - Security Code Review
Questions