My thoughts on Secure Code Review


Appsec / July 1, 2022 • 8 min read

Tags: Code Review


This article is going to cover my journey in cyber security, thoughts on secure code review and how to improve your own skills.

I have now worked for four years professionally in the cyber security field as consultant, mostly doing white-box assessments with access to source code. Since I have a developer background, I always approach a project with source code in mind, because to me, source is absolute truth. And from a customer perspective, it’s way more worth the money for me to be able to view the source in order to find deeper issues. I need to be able to read the code to comment on the application’s overall security posture.

During these years I have improved my skills in many ways, both in methodology, techniques and overall knowledge about systems and programming. You may ask “how”, well, through sheer motivation. Bug bounty and programming was a big part of my journey. Always reading the latest reports, saving everything of value then re-reading it later. Anything I could practice on I used, e.g., CTF and HackTheBox.

I regret that I didn’t have some sort of mentor, but nothing is impossible if you have the right attitude.

It’s important to be aware of your knowledge gaps and always improve where you can. Nasty bugs can exist in all kinds of places. If you are only looking in specific areas, you’ll miss all the bugs that exists outside of your comfort zone.

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.

Before we start, I want to also clarify that in order to be successful in secure code review, you need to be able to understand code, which you can only achieve by writing code yourself.

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 indication of unnecessary complexity that might contain vulnerabilities.

There are a few tools that can aide you in understanding the quality and complexity of an application, such as ABC Metrics and Cyclomatic complexity. I don’t personally use these types of methods when doing code review.

Using static analysis tools such as Semgrep and/or CodeQL in 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 is handling 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 flows myself, while I am testing. Having a debugger is of course preferable.

Identify 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 possible to exploit. 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 of the list and move to the next one. If you found something funky but couldn’t really exploit it, write this 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 which you would like to pursue. This is totally fine and is encouraged. The list produced during the brainstorming phase should not be considered as 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. Progressing in code review means understanding the code, be able to identify where problems can arise. Also, how easy it is for you to approach a new codebase.

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.

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 is 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 chose 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 processes 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 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 have found 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 for 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!