PA193 - Secure coding
principles and practices
Security Code Review
Łukasz Chmielewski chmiel@fi.muni.cz
https://drive.google.com/file/d/1KSlG9uI-BiavBrvODeiaXUge_ZeIrp2W/view?usp=sharing
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
• Access to source code doesn’t guarantee bugfree
code
• Usage of automated tools can provide great
advantage, but deep understanding of code
before change must remain
• Manual code review eventually spotted the
problem
6 | PA193 - Security Code Review
Why to perform source code review?
7 | PA193 - Security Code Review
SECURITY CODE REVIEW
8 | 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
9 | 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
• Certifications
– Independent labs
10
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 1,000 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
– helping you making better future estimations
12 | PA193 - Security Code Review
Information Collection
• Developer interviews
• Developer documentation
• Standards documentation
• Source profiling
• System profiling
• Reports from previous reviews
• History of changes
– For example, git log
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
– start from design specification
– establish threat model
– find design vulnerabilities first
– find logical implementation vulnerabilities second
– 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
• 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 initialized?
– 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?
– 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 in Trusted Elements / Hardware
• 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
• https://web.archive.org/web/20150329111329/https://www.praetorian.com/campai
gn/software-security/security-code-review.html
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
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
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
– search for interesting system calls
– use disassembling, or binary debugging
• Application-Specific Candidate Points
– patterns of mistakes for particular application
– learned from previous code/binary analysis
– e.g., new 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
46 | PA193 - Security Code Review
Handy tools
• Static and dynamic analyzers
– detect multiple issues
– 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
• Tools to find different instances of existing issues
– CodeQL
• 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
– https://ieeexplore.ieee.org/document/1668009
• Software Security Code Review
– https://web.archive.org/web/20100208065233/http://www.softwaremag.com/l.cf
m?doc=2005-07/2005-07code
• Performing security Review (Microsoft)
– https://web.archive.org/web/20141023191707/http://silverstr.ufies.org:80/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
– https://mfinifter.github.io/papers/coderev-essos13.pdf
51 | 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
• How to find 5 RCEs in Apache Struts with CodeQL
– https://securitylab.github.com/research/apache-struts-CVE-2018-
11776/
52
CONCLUSIONS
53 | 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)
54 | PA193 - Security Code Review
Questions