Skip to content

Hacktivity 2012 - Krisztián Schäffer

Security Code Review

CommunityHacktivity - IT Security Festival39 minIntermediateOpen on YouTube ↗

Security Code Review examines Hacktivity 2012 - Krisztián Schäffer.

Summary

### Overview In this Hacktivity 2012 presentation, code auditor Krisztián Schäffer discusses the role and practice of security code reviews and code auditing within software development. He notes that security code review is often treated like an unwelcome byproduct of development or a neglected quality assurance activity, particularly in commercial business applications where speed and low cost are prioritized over code quality. In contrast, large open-source projects like the Linux kernel, Firefox, and Google's Chromium integrate code review directly into their development lifecycles. Schäffer breaks down code review into three primary areas: vulnerability research, security code auditing methodologies, and building a security culture through quality assurance.

### Key Topics - **Code Review in Practice:** Examining peer review systems (such as Chromium's Gerrit-based model) where developers read delta changes, debate security and typing implications, and require supervisor approvals before committing code. - **Vulnerability Research vs. Weaknesses:** Distinguishing between actual exploitable vulnerabilities and underlying code weaknesses. Schäffer emphasizes that code auditors should focus on finding and eliminating bad patterns rather than spending excessive time proving whether an issue is immediately exploitable. - **White-Box Auditing vs. Black-Box Testing:** Comparing the strengths and trade-offs of black-box testing (behavioral testing, simulating attackers with limited time, building client trust) versus white-box code auditing (root-cause identification, better developer feedback, deep structural visibility). - **Auditing Methodologies:** Exploring practical code reading strategies, including control-flow sensitive and data-flow sensitive analysis, threat modeling with business stakeholders, developer interviews, and static analysis tools. - **Metrics and Security Culture:** Implementing metrics such as lines of code reviewed per hour (LOC/h), defect density, and detection rate, alongside secure coding standards and developer training.

### Technical Details - **MySQL Authentication Bypass Case Study:** Schäffer analyzes a major bug involving MySQL authentication and the C standard library `memcmp` function. The code compared password hash blocks using `memcmp` and assigned the return value to a `my_bool` type (defined as a signed `char`). While `memcmp` returns an integer (zero for a match, non-zero for differences based on byte comparison), casting non-zero results (such as multiples of 256 or values where the lower byte truncates to 0) into a signed byte can cause a non-zero integer to evaluate to zero, incorrectly bypassing authentication roughly every 256 attempts. He critiques automated tools (like Clang static analysis and Splint) and explains how static analyzers either missed the bug or suggested casts that silenced warnings without fixing the root cause. - **XML Injection via Session Cookies:** An example from an authorization module demonstrated an XML payload constructed by concatenating user session cookies directly with string addition (`+`). This allowed attackers to craft session cookie values containing injected XML tags to request unauthorized actions or resources. - **Java Class Loading Flaw:** A Java zero-day example illustrated fallback code where, after failing standard permission checks, the code attempted an alternative class-loading path, inadvertently allowing an app to load unauthorized classes. - **Custom Crypto Experiments:** The speaker references an example where custom, poorly understood code generation tools were introduced into a cryptography module, violating standard security principles against unapproved tools and experimental cryptography.

### Takeaways - **Audit Speed and Metrics:** Reviewing more than 400 lines of code per hour significantly reduces the likelihood of finding subtle security bugs. Teams should track defect density and detection rates. - **Don't Waste Time on Exploitability During Audits:** If a dangerous pattern or weakness (such as raw string concatenation into XML) is identified, flag it for remediation immediately rather than spending audit time proving exploitability. - **Pair Auditing as Training:** Sitting down face-to-face with developers to review code acts as an effective, hands-on secure coding training session. - **Early Static Analysis:** Static analysis should be integrated early; running it late produces unmanageable volumes of warnings, whereas using it incrementally educates developers on typing and coding standards. - **Adherence to Approved Tools:** Security-sensitive components (especially cryptography) must rely strictly on well-understood, approved tools and established coding codices rather than experimental approaches.

Related talks