www.crcs.cz/rsa @CRoCS_MUNI
PV286 - Secure coding principles and
practices
Security Code Review
Łukasz Chmielewski chmiel@fi.muni.cz (email me with your questions/feedback)
Centre for Research on Cryptography and Security, Masaryk University
www.crcs.cz/rsa @CRoCS_MUNI
Organizational Aspects
• Since there is only a video lecture, in case of questions:
– Email me chmiel@fi.muni.cz or
– Consultations are not Friday 9.30 – 11.00 in A406.
– The video from 2023 is uploaded just next to this presentation.
• Questionnaire will be open on Tuesday evening and it will remain
open till the end of Tuesday next week (23.59 on 14/05)
• For people following PA193 seminars:
– Please watch the lecture before your seminar
2 PV286 | Secure API, Automata-based programming
www.crcs.cz/rsa @CRoCS_MUNI
PROBLEM
3 | PV286 - Security Code Review
www.crcs.cz/rsa @CRoCS_MUNI
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!
4 | PV286 - Security Code Review
www.crcs.cz/rsa @CRoCS_MUNI
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
5 | PV286 - Security Code Review
MD_Update(&m,buf,j);
MD_Update(&m,buf,j); /* purify complains */
www.crcs.cz/rsa @CRoCS_MUNI
Fatal mistake
6 | PV286 - Security Code Review
www.crcs.cz/rsa @CRoCS_MUNI
Morale
• Access to source code doesn’t guarantee bug-free 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
7 | PV286 - Security Code Review
www.crcs.cz/rsa @CRoCS_MUNI
Why to perform source code review?
8 | PV286 - Security Code Review
www.crcs.cz/rsa @CRoCS_MUNI
SECURITY CODE REVIEW
9 | PV286 - Security Code Review
www.crcs.cz/rsa @CRoCS_MUNI
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
10 | PV286 - Security Code Review
www.crcs.cz/rsa @CRoCS_MUNI| PV286 - 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
11
www.crcs.cz/rsa @CRoCS_MUNI
Application review phases
1. Pre-assessment
2. Application review
3. Documentation and analysis
4. Remediation support
12 | PV286 - Security Code Review
www.crcs.cz/rsa @CRoCS_MUNI
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
13 | PV286 - Security Code Review
www.crcs.cz/rsa @CRoCS_MUNI
Information Collection
• Developer interviews
• Developer documentation
• Standards documentation
• Source profiling
• System profiling
• Reports from previous reviews
• History of changes
– For example, git log
14 | PV286 - Security Code Review
www.crcs.cz/rsa @CRoCS_MUNI
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
15 | PV286 - Security Code Review
www.crcs.cz/rsa @CRoCS_MUNI
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.
16 | PV286 - Security Code Review
www.crcs.cz/rsa @CRoCS_MUNI
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
17 | PV286 - Security Code Review
www.crcs.cz/rsa @CRoCS_MUNI
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
18 | PV286 - Security Code Review
www.crcs.cz/rsa @CRoCS_MUNI
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
19 | PV286 - Security Code Review
www.crcs.cz/rsa @CRoCS_MUNI
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?
20 | PV286 - Security Code Review
The Art of Software Security Assessment: Identifying and Preventing Software Vulnerabilities
www.crcs.cz/rsa @CRoCS_MUNI
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?
21 | PV286 - Security Code Review
The Art of Software Security Assessment: Identifying and Preventing Software Vulnerabilities
www.crcs.cz/rsa @CRoCS_MUNI| PV286 - 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
22
www.crcs.cz/rsa @CRoCS_MUNI
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
23 | PV286 - Security Code Review
www.crcs.cz/rsa @CRoCS_MUNI
Finding summary - example
24 | PV286 - 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
www.crcs.cz/rsa @CRoCS_MUNI| PV286 - Security Code Review
Architecture review
www.crcs.cz/rsa @CRoCS_MUNI| PV286 - 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?
26
www.crcs.cz/rsa @CRoCS_MUNI| PV286 - 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?
• Hint: always search for debug features…
– how they are disabled / turned off
27
www.crcs.cz/rsa @CRoCS_MUNI| PV286 - 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?
28
www.crcs.cz/rsa @CRoCS_MUNI| PV286 - Security Code Review
Protocol design (and implementation)
• Packet confidentiality, integrity and authenticity
• Packet removal/insertion detection
• Replay attack
• Reflection attack
• Man in the middle
29
www.crcs.cz/rsa @CRoCS_MUNI| PV286 - 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()?
30
www.crcs.cz/rsa @CRoCS_MUNI| PV286 - 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?
31
www.crcs.cz/rsa @CRoCS_MUNI| PV286 - 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?
32
www.crcs.cz/rsa @CRoCS_MUNI
CODE INSPECTION
33 | PV286 - Security Code Review
www.crcs.cz/rsa @CRoCS_MUNI
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/campaign/software-
security/security-code-review.html
34 | PV286 - Security Code Review
www.crcs.cz/rsa @CRoCS_MUNI
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
– See for example: https://codeql.github.com/docs/writing-codeql-queries/about-data-flow-analysis/
• Code navigation tools provide great help
– call graphs (Doxygen, Performance profilers)
– tainted values (e.g., taintgrind)
– ...
35 | PV286 - Security Code Review
www.crcs.cz/rsa @CRoCS_MUNI
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
36 | PV286 - Security Code Review
www.crcs.cz/rsa @CRoCS_MUNI| PV286 - Security Code Review
Code comprehension (CC) strategies
www.crcs.cz/rsa @CRoCS_MUNI
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”?
38 | PV286 - Security Code Review
www.crcs.cz/rsa @CRoCS_MUNI
Trace Malicious Input - characteristics
39 | PV286 - Security Code Review
The Art of Software Security Assessment: Identifying and Preventing Software Vulnerabilities
www.crcs.cz/rsa @CRoCS_MUNI
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
40 | PV286 - Security Code Review
www.crcs.cz/rsa @CRoCS_MUNI
Analyse a Module - characteristics
41 | PV286 - Security Code Review
The Art of Software Security Assessment: Identifying and Preventing Software Vulnerabilities
www.crcs.cz/rsa @CRoCS_MUNI
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
42 | PV286 - Security Code Review
www.crcs.cz/rsa @CRoCS_MUNI
CANDIDATE POINTS (CP) STRATEGIES
43 | PV286 - Security Code Review
www.crcs.cz/rsa @CRoCS_MUNI
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
44 | PV286 - Security Code Review
www.crcs.cz/rsa @CRoCS_MUNI
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
45 | PV286 - Security Code Review
www.crcs.cz/rsa @CRoCS_MUNI
TOOLS
46 | PV286 - Security Code Review
www.crcs.cz/rsa @CRoCS_MUNI
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
47 | PV286 - Security Code Review
www.crcs.cz/rsa @CRoCS_MUNI
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)
48 | PV286 - Security Code Review
www.crcs.cz/rsa @CRoCS_MUNI
ANTI-PATTERNS
49 | PV286 - Security Code Review
www.crcs.cz/rsa @CRoCS_MUNI| PV286 - 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!
50
www.crcs.cz/rsa @CRoCS_MUNI
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/
51 | PV286 - Security Code Review
www.crcs.cz/rsa @CRoCS_MUNI| PV286 - Security Code Review
Practical & Commercial Considerations
52
• Source Review in Certifications:
– Common Criteria, Composite Evaluations:
• https://www.commoncriteriaportal.org/iccc/7iccc/t2/t2201000.pdf
– Platform / Chipset Security Evaluation for DRM
– Riscure Assurance Levels (also a form of multi-staged evaluation)
• https://www.riscure.com/service/riscure-assurance-premium-content/
• Code Coverage
– Use Cases: boot code vs. operating system (e.g. smartcard vs. android) vs. application
– Metrics: issues per lines of code
• Types of issues: logical vs. side-channel issues
– What about timing and microarchitectural attacks?
• Hint: look at connections between hardware and software, for example:
– Initialization of trusted hardware components (e.g., TEE)
– Debug Interfaces, e.g., JTAG
www.crcs.cz/rsa @CRoCS_MUNI
CONCLUSIONS & READING
53 | PV286 - Security Code Review
www.crcs.cz/rsa @CRoCS_MUNI
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.cfm?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
54 | PV286 - Security Code Review
www.crcs.cz/rsa @CRoCS_MUNI| PV286 - 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)
– https://web.archive.org/web/20190821204156/http://www.owasp.org/index.php/Code_Revie
w_Introduction
• How to find 5 RCEs in Apache Struts with CodeQL
– https://securitylab.github.com/research/apache-struts-CVE-2018-11776/
55
www.crcs.cz/rsa @CRoCS_MUNI
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)
56 | PV286 - Security Code Review
Questions