Recently I was asked by someone what you should look for when attempting to perform a code review from a security perspective. I'm now going to say something perhaps rather controvesial, I don't believe in code reviews. There are several reasons why I have this opinion, which is the purpose of this blog.
Firstly I view code reviews to be a historical means to approaching security. If we look back to the past, it made perfect sense to do it back then. The code of applications was far more different to what we have today. Firstly many applications were built from the ground up. This meant that all the code was immediately available, or rather most of the code. Instead today, most applications are made from libraries and frameworks, where a lot of the code is abstracted away. Meaning that to understand what is happening you would need to be familiar with the library or framework. It is possible, but possibly means more time needed. On top of this, application were built in large chunks with big monolithic releases. This meant that someone could spend days reviewing code. Fast forward to today, we now have things like CI/CD (Continious Integration / Continious Delivery) where there is a constant stream of new code. This means we now need to be a lot more selective about the code that we review (since we simply don't have the resources to review all of the code).
The points above are minor and not particularly the reason I'm not a fan on relying on code reviews for security purposes. However, there are further points that are not so much minor in nature. Firstly, I've seen many things missed by code reviews over my years in the industry. The problem comes down to the fact that humans are not machines. Some people are great at carrying out code reviews (I'm certainly not one of them), where others aren't. Even those who are great at code reviews may have an off day, be it a tight deadline or something else on their mind, being distracted while carrying out a code review. Perhaps why I've seen so many things missed from code reviews. How many times have we seen significant vulnerabilities in open-source software (OpenSSL, Log4j, Spring Framework, etc.)? With some of these vulnerabilities being in the software for years!
Along with this, and secondly, do all those reviewing the code have all the technical knowledge required? In most organisations this could potentially become a bottleneck where only a few team members may have this appropriate knowledge. And this isn't just about secure coding practices, this could also be security related knowledge around a library, framework or even protocol. Put simply, manual code reviews will not scale at most organisations.
Thirdly, manual code reviews take time. They take time to figure out the logic that is happening, time to trace the process flow, time to understand the code, etc. As we push to have new features out to consumers faster and faster, this becomes a problem, and something must give. This is why automation is key. This can scale well and helps focus on the more mundane tasks (something which humans are particularly good at).
Instead, what I would recommend is white box security testing approach. I'm of the opinion that you would gain far better value for your time using this approach. Testing the running application, along with the tracing of code is a far better approach in my opinion. You can see in real-time what the results are. You can experiment with different inputs and see how the system reacts. Using features such as debugging (even remote debugging), allows you to see exactly what is happening. And with most debuggers you can even change values on the fly. If you then combine this with tools such as SAST and DAST scanners (you should already have this), you should then have a pretty broad coverage. Also leveraging tools such as Semgrep and develop signatures for known bad code.
In summary, I see manual code reviews as a legacy approach to the problem. They certainly had their value back when there was far less need for automation, as well as limited tooling available. As we move forward, automation has become key, so we need to adopt our strategies to ensure that we embrace and leverage automation as much as possible (DevSecOps anyone?). When resourcing becomes an ever increasing issue, we need to work smarter, not harder.
