Reviewing code for security is one of the important building blocks of ensuring software security. I consider code analysis, in general, to be a very powerful and universal way to get the most detailed information about a piece of software. It can be used in different situations, from catching bugs in a newly developed feature to deriving a security model of a third party application (if code is available, of course). In my work, I dedicate a considerable amount of time to reviewing code for security. I find that, while the purpose and scope of reviews differ depending on a situation, the general approach that I use to decompose and analyze code for security often stays the same. Here is a list of high-level things that I usually look for:
- Application entry and exit points -- understanding all possible intended and unintended ways data enters and exits the application (e.g., through APIs, user interfaces, and filesystem access) is the foundation for any further analysis
- Control and data flow -- also, intended and unintended
- Trust boundaries and their crossings
- Authentication and authorization enforcement on all entry points and trust boundary crossings
- Input validation on all entry points and output escaping when needed
- Protection of data at rest and in flight
- Permissions (filesystem, processes, etc)
- Injection points -- any change of context in which data is used creates possibilities for injections
- Any explicit and implicit assumptions done by the code and ways to violate them
- Any application domain-specific bad practices and vulnerabilities (such as CSRF vulnerabilities in web applications) -- this can be an art on its own and requires good understanding of the domain and known problems
- Any language or technology specific bad practices
Use of static analysis tools (if such are available for the target language) is another important component of doing a thorough code analysis. Static analysis tools usually achieve high and systematic coverage of all possible execution paths that is hard to ensure with human analysis. They range from tools looking for common known-to-be-bad patterns in the code to more sophisticated tools doing more context-sensitive analysis to identify more complex issues. Yet, it's important to understand that static analysis tools should be used as a complement, not a replacement, to human analysis because they suffer from various inherent limitations (which are outside the scope of this post) that allow them to discover only a subset of bugs that can be discovered with diligent human analysis. For example, identifying bugs that require understanding of program semantics or intended behavior generally requires human involvement.
If done properly and thoroughly, code reviews can be very time-consuming and tedious. However, they should never be skipped regardless of all other activities employed around software security (such as specification/design reviews, penetration testing, and education of developers). At the end of the day, security is always about details (it's about one thing gone wrong rather than everything done right) and code is a good place to discover some of the most subtle ones. I would even argue that given enough scrutiny, time, and human resources, most bugs in the system could be discovered just by looking at its code. In real life, of course, code reviews are time and effort bounded and finding the most potentially severe bugs in a reasonable amount of time is an art, requiring good understanding of the application's domain and its threat model.