A short insight to code review

Pratyaksh Singh
5 min readMay 7, 2023

--

Hey everyone!! I hope you are doing well.

Why this blog?

When I started gaining skills in penetration testing, the initial months of my learning involved copy-pasting the payloads and replicating what others did. I had no idea how to perform a pentest/hacking, this made most of my hacking a distasteful experience. I realized the obvious problem was my learning approach, I was too occupied with finding vulnerabilities but never cared to know about the reasons why a vulnerability exists.

To change my approach I started learning a bit of programming and started coding tools either in Python or Bash that I used daily. When I started my first internship, I insisted my seniors assign me static source code analysis projects. Not only my seniors assigned me the project, but they also helped me complete it successfully by guiding me throughout the project. While performing my first code review on an actual project, I for the first time understood why a vulnerability existed in an application. This experience gave me a perspective on how developers think and what mistakes they commonly make while programming, this is what got me started in reviewing codes and helped me become better at my skills.

I knew I wasn’t alone to go through this experience and many still do thus I decided to write this blog. Now without any further ado, let us jump right into the basics of it, or what I’ve learnt so far.

What is code review?

Secure code review is the process of examining an application’s source code to improve the overall security posture of the application. It can either be automated(language, framework-specific) or can be manual. The goal is not just to identify the security flaws within the source code but also to provide valid mitigation and to introduce secure code practices within the development cycle to prevent the preparation of insecure code implementation.

Benefits of learning source code review:

  1. Gives you a deeper understanding of how a developer thinks and how functionalities can be exploited.
  2. If you are a Pentester, you’d understand why a bug exists in the first place.
  3. It helps you find more bugs in real-life, now that you know how a developer thinks and implements a piece of code, you can find a workaround for the same functionality and exploit it.
  4. It helps you find sensitive info related to your target, acute instances like AWS keys, auth tokens, API keys, hardcoded credentials, exposed files, and more.

How to review code?

Before learning the proper way of reviewing code, in the initial days of my code review, I primarily focused on security misconfiguration and hard-coded sensitive information.

Consider, if my target code was based on ASP.NET, I was focused on the primary findings like:

1. Sensitive hardcoded info like credentials, API key, Secret tokens etc

Import the code you’re reviewing in any code editor(I prefer VS Code) and start grepping for keywords such as:

  • password / pwd / passwd / pass / password / admin_passwd / adminpasswd / adminPass
  • API_Key / apikey / api-key
  • Secret / Secrect_key / skey / private key / private_key / pkey

You can grep for more keywords by using a wordlist from GitHub. Here is one that I follow, https://github.com/random-robbie/keywords/blob/master/keywords.txt

2. Security misconfiguration

a. Custom Errors

Take the below XML as an example, here we are checking whether the ASP.NET code has implemented custom error handling, if not, sensitive information can be disclosed on the client side via stack trace or default errors.

  • Secure code:
<configuration>
<system.web>
<customErrors mode="Off">
  • Misconfigured code:
<customErrors mode="RemoteOnly"  
defaultRedirect="customerror.htm">
<error statusCode="404" redirect="customerror404.htm"/>
</customErrors>

b. HTTPOnly misconfiguration+

HTTPOnly is a cookie attribute that helps mitigate client-side scripts from accessing protected cookies. If disabled, any malicious client-side scripts can access these cookies to later escalate the security flaw. in ASP.NET within the web.config file, below is how you define the HTTPOnly cookie attribute.

  • Misconfigured code:
<configuration>
<system.web>
<httpCookies httpOnlyCookies="false">
  • Secure code:
<configuration>
<system.web>
<httpCookies httpOnlyCookies="true">

Similarly, you too can start reviewing codes by focusing on finding security misconfigurations within the code you’re reviewing based on the language. A simple Google search will help you find resources.

From the above examples, it may seem to some that code review is just grepping for keywords and eventually finding vulnerabilities. However, to an extent it is true but as for code review in its entirety, the above example will not take you far enough.

Reviewing code involves skills such as pattern recognition, programming, identification of vulnerable elements/functions and a lot more. Take the below example:

...
string userName = ctx.getAuthenticatedUserName();
string query = "SELECT * FROM items WHERE owner = '" + userName + "' AND itemname = '" + ItemName.Text + "'";
sda = new SqlDataAdapter(query, conn);
DataTable dt = new DataTable();
sda.Fill(dt);
...

The above is a SQL code snippet and it has nothing that has been discussed before in this blog. To review this code we will have first to understand what is happening here.

The above code dynamically constructs a SQL query wherein the query returns data where the “owner” matches the user name. Hereby performing a direct concatenation of user input “userName” with the SQL query rather than using parameterized statements to eliminate any SQL injection vulnerabilities.

Ideally, the above SQL query should be built as follows:

SELECT * FROM items WHERE owner = {{username}} AND itemname = {{itemName}};

Although an attacker can manipulate the above query to perform an SQL Injection attack here by breaking the SQL query by entering the below-mentioned payload as the value for the itemName user input

payload' OR '1'='1

The above statement would make the final query as:

SELECT * FROM items WHERE owner = 'Attacker' AND itemname = 'payload' OR '1'='1';

The OR '1'='1 makes the SQL query perform in an unintended manner as now the usage or OR condition and providing a True condition i.e. '1'='1' makes the query return all the data regardless of their owners.

The fault in implementation makes the code snippet vulnerable to SQL injection attacks.

So, the question arises is how do you review such codes? How do you review a source code with millions of lines of code? It might seem overwhelming but trust me there are multiple efficient ways to review code. We will discuss more about code review in the coming parts of this series. Until then you can go through a Twitter thread I posted for learning source code review which you can find here.

I’ve kept this blog short and simple to give you an insight into what code review can be and how you too can get started with it. I’ll share more with you guys soon.

## References:

You can find me here!!

--

--

Pratyaksh Singh
Pratyaksh Singh

Written by Pratyaksh Singh

Security Reasearcher | Pentester | ThinkTank | OOBT | Source code Reviewer | Android PT

Responses (1)