In an agile world, security can be a time consuming effort that slows down the software delivery processes. However, security topics should not be ignored or shifted to the very last stage of the software development lifecycle. Especially when the security team has just implemented a vulnerability scanning tool, the results can be overwhelming. Vulnerabilities and bad security practices can be anywhere in the code and pipeline to production. You can speed up the delivery of your applications by applying the principles of shifting left. Proper code reviews play an important role to increase security. In this article I would like to share code review tips and tricks to improve security.
When to review code
Following the shift-left principles of security, it’s advised to review code for security issues as early as possible. This reduces the time needed to correct any security related issues. Ideally code reviews should happen before merging code changes into the mainline (master) branch of your source code control system. More advanced teams can even adopt the principles of Trunk Based Development. It’s much more costly to fix security issues after the release of a new version: developers need to stop working on their current task and shift their mind to the requirements of the feature that has the security flaw. As they fix the issue, the new code also needs to be checked in, processed, reviewed and a new version of the application built. Context switching between old code with security flaws and features they’re currently working on is very bad for productivity, as well as quality of both the old and new code.
Another phase for code reviews are the CI/CD pipelines that pushes the code from development to production. Add quality gates as “roadblocks” that stop the deployment of code that has severe security vulnerabilities or other bad practices.
Every organization has unique quality gates that align with their way of working. Furthermore, the quality gate depends on the existing security related knowledge within the teams. Also it should reflect the level of risks an organization is willing to take.
One of the last stages of code reviews are the postmortem situations. In those situations, the team looks back about a critical error or problem and they learn from it. Sometimes it takes a lot of effort to convince the management why these ceremonies are important to do code reviews. The goal is to control the risks of deploying bad code into production while staying agile at the same time.
Focus
As mentioned in the beginning of the article, potential security flaws can be everywhere in the code. The important question is: where should I start and what should I focus on? It’s a big risk not to look at critical security related components.
What
From a pure source code perspective, these tips can help you to select what you should focus on when performing code reviews:
- Check the authentication and authorization components. Also think of crypto functions and components which use and/or implement those crypto functions.
- Source code which handles highly private & confidential data or which handles transactions and/or “digital money” (e.g. credits, coins, etc)
- The same is true for any API which deals with the topics in the previous bullet point.
- Public endpoints which expose services to the internet Also think of file upload components in which (anonymous) users can upload files to your application.
- New code. The risk even increases when a complete framework is being used for the first time. Developers are not (so) familiar with it yet, so they most likely do not know the ins and outs of it yet.
- Take extra care of code written by new or inexperienced team members.
Who
Peer reviews are a good practice for a lot of user stories. Two persons working together at the same user story will help each other to write better code which is according to the stated requirements. You can utilize this for security reviews as well.
Having someone from the same team review your code is faster and (probably) more reliable than having an external person reviewing your code who does not know your application very well. Main reasons are: the team member does not need to learn the application domain and they do not need to get the right context for the source code. Also, you don’t need to wait hours or days for an external person to become available, since both are working at the same project team. It’s best to do the reviews in the current sprint to finish up the user story in time.
That being said, it’s advised to carry out the security reviews by an experienced developer. A junior developer might miss critical security issues. Besides this, the experienced developer can also quickly scan the code and find other bad practices. At best he gives proper advice the any junior developer to increase his knowledge. In the end this builds up the collective understanding of security issues, a better code base and more reliable software which is delivered faster.
Another advantage of peer reviews is to reduce the likelihood of introducing insider threads. This is like back-doors which a developer creates (un)intentionally. Also, think of pieces of debugging code which should be removed before shipping to production. Improper error handling could also lead to security risks since it reveals too much information in case a hacker tries to exploit the system.
Refactoring
In one of my previous posts, I emphasized the importance of refactoring sloppy source code. From a security standpoint this even becomes more important. Clean code is important for the following reasons:
- If source code is not according to coding standards or needs heavy refactoring it’s more difficult to find the real problems. Reviewers are distracted by the bad code and complexity and they quickly overlook any important security issues.
- In case reviewers aren’t so familiar with the application (domain) they need to read more code and other sources of information to get the right context for the components which they are reviewing. They quickly run out of time before they can find any real problems.
- In case the application is scanned by a security tool, include the analysis results as comments in the source code. This helps the reviewer to judge the scanning tool and to focus on other aspects. It also increases the awareness of anyone touching the code.
Properly refactored code is easier and safer to change since it reduces the likelihood of introducing new risks. One of the best examples of this is the patching of the OpenSSL heart-bleed bug. It took the OpenSSL team several months to refactor the code before they could fix the critical vulnerability. All the time the vulnerable package remained vulnerable.
Automated code reviews
While manual code reviews offer a good way to spot security risks and vulnerabilities, automated tools can help you speed up to deliver better software:
- People are human, and they make mistakes. By codifying knowledge into a system, adding and improving the tests and scans continuously, people can concentrate on improving the quality of the tools, and let the tools do the scanning.
- In case a team needs to adhere to certain compliance regulations, an automated code review tool can easily scan the code against these regulations. For example: a lot of security tools can scan source code based upon PCI DSS or SOX regulations.
- If your organization does not have so much security skills “in house”, these tool can help you find common security bugs and other vulnerabilities in a fast way.
- It’s relatively easy to implement security tools in your CI/CD pipelines and to set acceptable thresholds. Your pipeline should break in case these thresholds are not met. No debate or lengthy discussions over the outcome.
- Automatic tools are much more consistent than human beings, they never get tired of viewing the same code over and over again. Let alone, they do not complain about boring repetitive tasks 🙂
- When you have a large (legacy) code base, automated tools are the only way to get insights into the quality and security of this code base. It’s impossible to do this using manual code reviews on a “user story” base. Perhaps your legacy code is like a minefield…
Code analysis tools
In addition to automated code review tools, code analysis tools can also help to find common code mistakes and enforcing good coding practices. This helps the human reviewers to speed up their review processes. Code analysis tools help to find the following issues:
- Improper or missing data validation and injection vulnerabilities. For example: check the length or expected pattern of input of an online form.
- Weak implementations of common patterns like random number generators, ciphers or keys and common mistakes in security functions.
- Common mistakes in (server) configuration like insecure (HTTP only) endpoints, insecure HTTP headers, etc.
Tips and tricks
Automated code review should be reliable and help the developer teams. For this to happen, it’s good to keep the following tips in mind:
- Don’t just scan all of the source code using the tool and dump a report to the developers. Often developers see security topics as being a bottleneck for their day-to-day work. Have you already heard of “SecOps¨? 🙂 Yet another new buzzword which is getting more popular. Developers should embrace security if you want it to be improved.
- Never blindly trust any tool. Results vary greatly for individual tools. A lot of false positives are likely to popup. You don’t want developers and/or code reviewers to (constantly) check for the same false positives. Better sort out the results first and help developers adopt best practices (by supplying them with examples) to fix security issues. Share false positives across the organization. Other teams learn from it as well.
- Let the security team first experiment with the tools and set reliable rules. Store those rules “as code” in order to reset a given state. Auditors also demand predictability over how security topics are being measured.
- Estimate how long a scan of a typical application takes, so you know what to expect. If you don’t know the time it takes to do a full scan of an application in a CI/CD pipeline, your developers might be surprised that delivering a code change takes much longer than expected. They might hate the tool immediately.
- Be sure to change the default settings of the security review tool of choice. A tool should always be customized for the organization and the applications which are created by the developers. Leaving the defaults in place can generate a false sense of security or overwhelm both security teams and developers with inappropriate results.
The downside of tools
Tools are an addition to your manual code reviews. It’s good to keep in mind that tools also have their limitations and drawbacks. For example: a tool can’t tell you if you need to encrypt or sanitize sensitive certain data or if you should call a specific function for your application logic. This is all because tools are not context-aware. Thus tools can’t replace the manual reviews of experienced reviewers entirely.
Key take-aways
With the tips and tricks in this article I hope you acknowledge the usefulness of (proper) code reviews. I hope this will help you to incorporate code reviews to improve the security of your applications. In the end it also leads to better software which is delivered faster and more reliable. Before we conclude this article, let’s sum up some last key take-aways:
- Use a security scanning and testing tool to do the grunt work for you. Don’t let repetitive toil take up your developer’s time.
- It’s best to let developers do the code reviews and don’t completely depend on security teams. Build security awareness in into the Agile way of working. Those teams should come into play only for high-risk security code. Or judge the security frameworks.
- Perform code reviews in small, quick iterations. Reviewing code for more than one hour reduces the attention of the reviewer and it is more likely he/she will miss important aspects.
- Don’t just review the core application code. Also review the configuration for infrastructure components, comments, TODOs and other configuration aspects. In the Agile world everything is code, so everything which is relevant should be reviewed for security issues.
- Developers should feel safe about their code. It’s very important to give proper feedback about the code. In the end, this will help to create a better (shared) code base and a happier team which takes on security seriously.
