My thoughts on Secure Code Review

Appsec / July 1, 2022 • 7 min read

In this article I would like to share my thoughts, methodologies and techniques on how I perform secure code review. By secure I mean code review with the purpose of finding unknown vulnerabilities. My focus is generally on web applications, but the ideas mentioned below apply to other types of software as well.

1. Understanding 👨‍🏫

Begin by understanding the application. Browse through the code and the application’s functionality until you have a clear intuition for what the application is capable of. By browsing the code, you get a feel for the quality and style of the code. By quality I mean: is there a lot of duplicate code, how are variables and functions named? Is there a lot of over-engineering going on? This may be an indication of unnecessary complexity that might contain vulnerabilities.

There are a few tools that can aid you in understanding the quality and complexity of an application, such as ABC Metrics and Cyclomatic complexity. I generally do not use these types of methods when doing code review.

Using static analysis tools such as Semgrep and/or CodeQL at the beginning of the review can help you highlight possible security issues which you later can analyze. It can also give you a general feeling for what types of issues exist within the codebase.

Understand where all the user inputs are and how they are being consumed.

2. Brainstorm 🧠

Identify possible weak points, where can issues arise? For instance, if oauth is being used, how closely are they following the RFC? Do they verify state/nonce/signatures?

Is there login functionality, if so, how are sessions handled? If you know the application is using a well-established framework that handles sessions, you can mark that as “unlikely to be vulnerable”. But you should still verify that the application handles sessions correctly. This is easiest done by testing it in practice. You can follow the code flow as you test either with a debugger or doing it manually. I usually “trace” the function flow myself, while I am testing. Having a debugger is of course preferable.

Identify the technology stack. Are there any known vulnerabilities? Is OWASP-Dependency check used? If so, you got free information! Otherwise, you can build or find a tool that automatically checks for known vulnerabilities in dependencies.

Write down all your thoughts about what you have seen so far, and what you think could be exploited. My notes usually look something like this:

 1# Ze Notes
 2- [ ] Possible to convert .docx to pdf => XXE, LFI, RFI ?? Extract data from AWS metadata url?
 3
 4- [ ] User roles => check for broken access control
 5
 6- [X] WYSIWYG editor, inject xss? Works as expected
 7
 8- [-] eval() is used in src/path/file.py:45 => looks like user input can reach this, verify. => No dice
 9
10- [X] Outdated dependencies, check for known vulns => found CVE-123-456

X indicates I was successful, while - means unsuccessful.

3. Hack away 👩‍💻

Once you have brainstormed potential risks and possible issues, it’s time to work through them. Once each item has been verified, cross it off the list and move to the next one. If you found something funky but couldn’t really exploit it, write it down next to the item. Specify what you did and why you think there may be something funky going on, something that could lead to security vulnerabilities. This will help you when you come back to this item after a few hours or days.

Of course, you may identify other leads during testing that you would like to pursue. This is totally fine and is encouraged. The list produced during the brainstorming phase should not be considered final. New ideas will form as you get deeper into testing.

How to measure success 🤩

When reviewing open source applications or even customer applications, there will be times where you will find nothing. Sometimes, this is the case, there is nothing to be found. Therefore, it is important not to measure success by the amount of vulnerabilities found. Instead, measure success by your level of understanding of the codebase. If you understand the codebase, you can be comfortable in saying that there are no vulnerabilities.

Speed reading code is not a good indicator of progress. Code review is about quality, not quantity. It is not uncommon for reviewers to spend many hours reading the same code repeatedly. Good progress with lead to greater insight of the code you are reading, as well as improving how easy it to approach a new codebase.

It’s also worth mentioning that testing an application and reading code is quite exhausting. Therefore, it is important to take breaks, get some exercise and reload your mind every now and then. Looking at code while being tired as fuck is not going to yield great results.

Choose your style 🕺

You might have noticed that I work with checklists, and I follow a specific process. This might sound boring and restrictive. But the truth is that when you are testing an application for security vulnerabilities, you are essentially following a list. Either the list resides in your head, perfectly reserved, or you have an excel document. I personally use OWASP ASVS as my checklist in case I forget to test something. But I also rely on personal experience. Keep in mind that the ASVS is not a complete list. I simply use it as a reference for testing certain areas, such as “Authentication”. For example, let’s say that I for some reason forgot to test authentication issues. I would look in ASVS’s table of contents and immediately recognize that I forgot to test “Authentication”. Now, how I go about testing that is a different story. The individual testing process for a specific area, is usually based on previous experience and knowledge. Unless it is something complex, I rarely look up how to test a specific vulnerability class.

Oauth, OpenID Connect and SAML is something I must read up on before a test/review because there are so many things going on and different flows you need to consider. Some things I just can’t keep in my head 🤷‍♂️

There are many ways you could go about testing an application with access to source code. You can for example choose a specific functionality and its corresponding source code. Maybe you feel confident you can simply read the code for a given function and see if there are any vulnerabilities, without spending too much time performing practical tests.

I personally use checklists and practical testing, assisted by source code. But how I choose to go about testing a specific software, changes from project to project. I might start with learning the application, then spend a day looking at code. Or the opposite.

The process is always changing, but the core methodology is always present.

Always improve 👩‍🎓

I mentioned before that it is good to always keep improving, sharpening the blade, developing the craft. By this I mean, reflect on your process, methodology and imagine other ways of doing it. Is there a part or technology in your process that you can update or improve? If you have an idea, sketch it out, try to develop a prototype and see if it works. It can literally be anything you find valuable.

For example, I created a program in Python that presents a few technologies that I mostly encounter. I select the ones that are relevant for the current project, in return I get all my notes specific to those technologies. Works surprisingly well. It’s faster than looking through all my files manually. I have some other ideas that I am going to develop to improve collaborative secure code review.

Point is, live on the edge and keep pushing boundaries. Develop new techniques and expand your mind.

Final thoughts 🚀

I hope you find this interesting and perhaps even adopted some of the methods detailed here.

Something I haven’t mentioned yet is, surround yourself with hackers that are better than you. Since joining my current employer, my motivation and interest in hacking has only amplified. Before I was alone in my quest for deeper knowledge, now I don’t have to do it alone.

Some general tips:

Ask questions, never assume, always verify
Documentation/RFC is your friend, probably your best friend
Code review takes time and consumes a lot of energy, make sure to take breaks and focus only on your current objective
Work in batches/chunks
Use SAST tools to uncover possible security issues to explore
Have a journal where you can write about your testing and any results you might have. You can also include issues you encountered during the review, such as the dev team destroying your environment which slowed down your testing. Or maybe you needed access to the source of a specific component but never got it (or got it very late).
Have a checklist containing possible issues that you identified during your brainstorm session. Work through these.
OWASP ASVS is your friend as well. It is impossible to remember every attack vector out there. Try to create a personal database of general issues as well as specific programming or framework issues. It should be searchable
If possible, save any important files such as swagger.yml or store all endpoints being tested
Risk assessment and context! Some vulnerabilities are not critical because of the context they are in.
Sources and sinks!