Code Reviews James Walden Northern Kentucky University Topics

Code Reviews James Walden Northern Kentucky University

Topics 1. 2. 3. 4. Types of Reviews Code Review Process Checklists Prioritizing Code to Review CSC 666: Secure Software Engineering

Code Reviews Inspection of source code by one or more people who aren’t the author of the code. § Goal: Identify defects for later removal. § People: Moderator, reviewers. § Scope: Module or small set of classes. § Time: 1 -2 hours; <1 kloc CSC 666: Secure Software Engineering

Benefits of Code Reviews 1. Find defects sooner in the lifecycle. (IBM finds 82% of defects before testing. ) 2. Find defects with less effort than testing. (IBM—rev: 3. 5 hrs/defect, test: 15 -25 hrs/defect. ) 3. Find different defects than testing. (Can identify design and requirements problems too. ) 4. Educate developers about security bugs. (Developers frequently make the same mistakes. ) CSC 666: Secure Software Engineering

PCI DSS 6. 3. 7. b Obtain and review policies to confirm that all custom application code changes for web applications must be reviewed (using either manual or automated processes) as follows: � Code changes are reviewed by individuals other then the originating code author, and by individuals who are knowledgeable in code revie techniques and secure coding practices. � Code reviews ensure code is developed accordin to secure coding guidelines such as the Open Web Security Project Guide (see PCI DSS Requirement 6. 5). � Appropriate corrections are implemented prior to release. � Code review results are reviewed and approved by management prior to release. CSC 666: Secure Software Engineering

Inspections § Most formal process. § § Thorough coverage with separated roles. Use checklists to focus on specified goals. Collect metrics to track defects. Determine whether further inspections of revised software needed at end of meeting. § Extensive documentation of effectiveness. § One study found 16 -20 defects/kloc with inspections compared with 3 defects/kloc in informal walkthrough. CSC 666: Secure Software Engineering

Roles in Reviews 1. Moderator Manages meeting; follows up on issues. 2. Reader Paraphrases code during meeting. Not the author. 3. Recorder Records bugs discovered. 4. Author Provides context for code; answers questions. Makes corrections after code review. CSC 666: Secure Software Engineering

Walkthroughs § Less formal process. § Author leads meeting and describes code. § Focus on author needs, not quality goals. § No checklists used or metrics gathered. § Quality varies widely. § Walkthrough quality depends solely on author. § Useful for educating developers about code, provide high level view of design and defects. CSC 666: Secure Software Engineering

Code Review Process Planning Prep Meeting Rework Follow-up Author Moderator Everyone Author Moderator CSC 666: Secure Software Engineering

Planning Prep Meeting Rework Follow-up Author Moderator Everyone Author Moderator 1. 2. 3. 4. Author initiates Planning once code ready. A Moderator is assigned to inspection. Author and Moderator assemble inspection pkg. Moderator identifies other participants. CSC 666: Secure Software Engineering

Preparation Planning Prep Meeting Rework Follow-up Author Moderator Everyone Author Moderator 1. Reviewers examine inspection package. 2. Reviewers use checklists and analysis tools. 3. Reviewers mark bugs found. CSC 666: Secure Software Engineering

Meeting Planning Prep Meeting Rework Follow-up Author Moderator Everyone Author Moderator 1. Reader describes code in own words. 2. Reviewers comment and ask questions. 3. Recorder notes all potential bugs, suggestions. 4. Team appraises code at meeting conclusion. CSC 666: Secure Software Engineering

Rework Planning Prep Meeting Rework Follow-up Author Moderator Everyone Author Moderator Author addresses issues recorded at meeting. CSC 666: Secure Software Engineering

Follow-up Planning Prep Meeting Rework Follow-up Author Moderator Everyone Author Moderator 1. Moderator meets with Author about rework. 2. Moderator verifies all changes made correctly. 3. Author checks in corrected code. CSC 666: Secure Software Engineering

Formality Spectrum Review Planning Prep Meeting Rework Followup Inspection Yes Yes Yes Team Review Yes Yes No Walkthrough Yes No Pair Yes Programming No Continuous Yes Peer Deskcheck Ad Hoc Review No Yes Possibly Yes No No No Yes No CSC 666: Secure Software Engineering Yes

Code Review Tips 1. Know your limits. Typical review speed is 150 -200 lines/hour. Limit meeting length to 1 -2 hours. 2. Know what bugs to look for. Checklists Static analysis tools 3. Use tools. Simple tools: grep, findstr Code viewers Static analysis tools 4. Require preparation before the meeting. CSC 666: Secure Software Engineering

Checklists Security reviews should include checklists of common problems, including: 1. 2. 3. 4. 5. 6. SQL injection Cross-site scripting Input validation bugs Checking return values Resource name canonicalization Race conditions CSC 666: Secure Software Engineering

Code Review Problems 1. Requires substantial expertise in area of programming and security to be effective. 2. Human readers are fallible and will miss mistakes. 3. Code reviews are slow. Unreviewed legacy code will take time to review. CSC 666: Secure Software Engineering

Prioritizing Code If you can’t review everything, review § Code that runs with privileged mode. § Code that listens on globally accessible sockets. § Code that is accessible w/o authentication. § Code with a history of vulnerabilities. § Code that handles sensitive data. § Complex code. § Code that changes frequently. CSC 666: Secure Software Engineering

Reviewing for SQL Injection CSC 666: Secure Software Engineering

Ex: Zune infinite loop on 12/31/08 year = 1980; while (days > 365) { if (Is. Leap. Year(year)) { if (days > 366) { days -= 366; year += 1; } } else { days -= 365; year += 1; } } CSC 666: Secure Software Engineering

Key Points § Roles § § Moderator Reader Recorder Author § Process § § § Planning Preparation Meeting Re-work Followup CSC 666: Secure Software Engineering

References 1. 2. 3. 4. 5. 6. 7. Brian Chess and Jacob West, Secure Programming with Static Analysis, Addison-Wesley, 2007. Michael Howard, “A Process for Performing Security Code Reviews. ” IEEE Security & Privacy, July 2006. Eoin Keary et. al. , OWASP Code Review Guide 1. 1, http: //www. owasp. org/index. php/Category: OWASP_Co de_Review_Project, 2008. Steve Mc. Connell, Code Complete, 2/e, Microsoft Press, 2004. Gary Mc. Graw, Software Security, Addison-Wesley, 2006. PCI Security Standards Council, PCI DSS Requirements and Security Assessment Procedures, v 1. 2, 2008. Karl Wiegers, Peer Reviews in Software, Addison. Wesley, 2002. CSC 666: Secure Software Engineering

Static Analysis James Walden Northern Kentucky University

Topics 1. 2. 3. 4. Why Static Analysis? False Positives and Negatives Static Analysis Internals Using the Tools CSC 666: Secure Software Engineering

What is Static Analysis? Static = without program execution § Includes everything except testing. § Usually used to refer to compiler type tools. Examples § Static type checking § Vulnerability detection tools § Formal methods CSC 666: Secure Software Engineering

Why Static Analysis? 1. Code reviews require substantial expertise in secure programming. 2. Human readers are fallible and will miss mistakes. 3. Code reviews are slow. Unreviewed legacy code will take time to review. CSC 666: Secure Software Engineering

Assurance Verification Techniques Formal Verification Static Analysis Code Review Security Testing Penetration Testing Cost CSC 666: Secure Software Engineering

False Negatives and Positives False Positives § Tool reports bugs in code that aren’t there. § Complex control or data flow can confuse tools. False Negatives § Tool fails to discover bugs that are there. § Code complexity or lack of rules to check. CSC 666: Secure Software Engineering

False Negatives and Positives Mistakes False Positives False Negatives Check Heuristics CSC 666: Secure Software Engineering

Static Analyis Approaches 1. Standard compiler warnings and type checking. 2. Lexing source checkers that look for bad names like strcpy() and gets(). 3. Parsing source code checkers. 4. Parsing checkers with annotations. 5. Formal proof based program verification. CSC 666: Secure Software Engineering

Static Analysis Internals § Parser § Model Builder § Analysis Engine CSC 666: Secure Software Engineering

Parser § Convert programming language to AST. § Must have a parser for each language that tool supports. Abstract Syntax Tree CSC 666: Secure Software Engineering

Control Flow Graph if(a > b) n. Consec = 0 s 1 = get. Hex. Char(a) s 2 = get. Hex. Char(b) return n. Consec CSC 666: Secure Software Engineering

Data Flow with SSA Source Code: if (bytes. Read < 8) { tail = (byte) bytes. Read; } SSA Form: if (bytes. Read 1 < 8) { tail 2 = (byte) bytes. Read 1; } tail 3 = φ(tail 1, tail 2); CSC 666: Secure Software Engineering

Taint Propagation Track flow of data from source to sink. § Source: where data comes into program. § Sink: function that consumes the data. Vulnerabilities reported if § Data comes from an untrusted source. § Data consumed by a dangerous sink. § No function between source and sink makes the data safe. CSC 666: Secure Software Engineering

Tainting SQL Injection Example $link = mysql_connect($DB_HOST, $DB_USERNAME, $DB_PASSWORD) or die ("Couldn't connect: ". mysql_error()); Source mysql_select_db($DB_DATABASE); $username = $_GET[‘username’]; Source $password = $_GET[‘password’]; $query = "select count(*) from users where username = '$username' and password = '$password'"; $result = mysql_query($query); Sink CSC 666: Secure Software Engineering

Local vs. Global Analysis Local Analysis: Analysis of an individual function, a. k. a. intraprocedural analysis. Global Analysis: Follows control and data flow between functions, a. k. a. interprocedural analysis. CSC 666: Secure Software Engineering

Rules Security knowledge base for tool. § § § Identify data sources. Identify data sinks. Model behavior of validation functions. Check for dangerous configurations. Check control flow (i. e. every lock released. ) Customize for process + project § Check coding style is obeyed. § Check for custom functions, standards. CSC 666: Secure Software Engineering

Static Analysis Tools Simple search (lexing) tools § Flawfinder § ITS 4 § RATS Parsing Tools § § § Fortify Source Code Analyzer Coverity Prevent Klocwork K 7 Suite Find. Bugs splint CSC 666: Secure Software Engineering

Using the Tools Who runs the tools? § Developers § Security team When do you run the tool? § § While code is being written (IDE integration) Before code check-in After each build After major milestones What do you do with the results? § Support code review process. § Support security metrics. § Use to decide if project should be released. CSC 666: Secure Software Engineering

Code Reviews Review Results Review Code Run Tool Update Rules Fix Bugs CSC 666: Secure Software Engineering

Static Analysis Metrics § Vulnerability density (vulns/KLOC) § Vulnerabilities divided by severity § Critical, high, medium, low § Vulnerability types § Injection, XSS, race conditions, etc. § Vulnerability dwell § How long bug remains in code after detection. § Audit coverage § Percentage of code covered by reviews. CSC 666: Secure Software Engineering

Evolution of a Single Project CSC 666: Secure Software Engineering

Key Points § Static Analysis § Source code needed, but not execution. § Fast, repeatable, objective analysis. § Sources and Sinks § Malicious input enters program via sources. § Exploitation occurs at the sink. § Mistakes § False positives § False negatives CSC 666: Secure Software Engineering

References 1. Brian Chess and Jacob West, Secure Programming with Static Analysis, Addison. Wesley, 2007. 2. Eoin Keary et. al. , OWASP Code Review Guide 1. 1, http: //www. owasp. org/index. php/Category: OW ASP_Code_Review_Project, 2008. 3. Gary Mc. Graw, Software Security, Addison. Wesley, 2006. 4. PCI Security Standards Council, PCI DSS Requirements and Security Assessment Procedures, v 1. 2, 2008. 5. Karl Wiegers, Peer Reviews in Software, Addison-Wesley, 2002. CSC 666: Secure Software Engineering