Secure Code Review: A Practical Approach

Application Security Training

The best web app pen-testing course on the market!

Skillset

This article is about different code review techniques and their application in the real world

What you will learn:

What is secure code review and how to deal with them in real-life scenarios

What you should know:

Basic security concepts

Secure Code Review:

Secure Code Review is a process which identifies the insecure piece of code which may cause a potential vulnerability in a later stage of the software development process, ultimately leading to an insecure application. When a vulnerability is detected in earlier stages of SDLC, it has less impact than the later stages of SDLC – when the insecure code moves to the production environment. In the SDLC (Software Development Life Cycle) process [Figure-1], the secure code review process comes under the Development Phase, which means that when the application is being coded by the developers, they can do self-code review or a security analyst can perform the code review, or both. The developers may use automated tools which can be integrated with their IDE (Eclipse, MS VS, etc…) and can do coding and code review simultaneously. After that, they can take the help of a security expert to identify more flaws in the code, as the security experts have a more intrinsic view of security issues which might be missed by the developers.

Different studies and surveys shows that approximately 75% of attacks happen due to an insecure application, inside which includes insecure code. This way, it becomes a very essential part of SDLC which should be performed rigorously. Developers mostly tend to focus on the functionality of the application and ignore the secure coding approach. But nowadays they have become more conscious about code review due to the increasing incidents of hacking and server attacks.

Figure – 1

Techniques to secure code review:

Generally, we can divide the secure code review process into two different techniques:

Automated tool based/ Black Box: In this approach, the secure code review is done using different open source/commercial tools. Mostly developers use them while they are coding, but a security analyst may also take help of them. Tools are very useful while doing code review when we implement the secure SDLC process in the organization and provide the tool to developers themselves to do a “self-code” review while they are coding. Also, the tools are useful in analyzing large codebase (millions of lines). They can quickly identify potential insecure pieces of code in the codebase, which may be analyzed by the developer or a security analyst.

Manual/ White Box: In this technique, a thorough code review is performed over the whole code, which may become a very tedious and tiresome process. But in this process, logical flaws may be identified which may not be possible using automated tools, such as business logic problems. Automated tools are mostly capable of finding technical flaws such as injection attacks but may miss flaws like authorization problems. In this process, instead of going line by line through whole code base, we can concentrate on potential problems in the code. Those potential vulnerabilities can be given a high priority. For example, in C/C++, if we try to find any copying function in the code and check whether it’s using functions such as, strcpy() for performing copy function. As we know, strcpy() is known to be vulnerable to buffer overflow attacks. We may also want to check if any customized encryption is being used in the application, which automated tools may miss as they can identify standard algorithms only.

So the best approach will be a mix of both, depending on the volume and criticality of data. In today’s world where many complex applications are developed, we can’t ignore any of the above mentioned techniques.

Benefits of Secure code review:

There are some factors that should be taken into consideration while developing a robust access control mechanism in the web application:

Effort benefit: The effort to fix the vulnerabilities in the earlier stage of the SDLC process is much less than the later stage of the process. Once the code is complete and the flaw is not identified, it’s a very tedious and time consuming process to find problems once the application is ready to move into production. Also, last minute fixing may affect the entire functionality of the program and hamper deadlines set for product release. Who knows, it may create another security flaw, which can easily be possible with a large and complex code.

Cost benefit: Cost is directly proportional to effort required. Not only development cost, but also, a vulnerability identified in the production environment may involve more costs. Again, it’s well worth it, as the costs associated with an attack can be much steeper.

Compliance: Some compliance, such as PCI, makes it necessary to do a secure code review before launching the product. So an organization following complete SDLC has better chance of being certified.

Reputation: Secure code review removes most of the security flaws in the earlier phase, making it more secure than just doing black box assessments. So there is less chance of the product being compromise, hence lesser chance of reputation damage.

Approach:

These are based on mix of standard process and my own approach. It may differ from person to person.

Standard process [Figure-2]:

Figure-2

Define scope: First, you need to understand and come up with a rough estimate about the scope of the code review and the efforts involved in it. Also, budget constraints may be defined. What type of code review may be performed? Black box or white box? Try to understand the business logic of the application. Remember which vulnerabilities you need to look for, such as OWASP Top 10, SANS, etc… One can try to review them as much as possible, if not all of them. Then you can deduce how many of them can be detected using tools and which are best suited for manual review.

Categorize the vulnerabilities: What is your priority, meaning what type of vulnerabilities you will take as a priority? For example, in business applications, you may concentrate mostly on the business logic of the application and dive deep into it, as the technical ones are easily detected by tools. Prioritize them.

The following are a few categories you can look at:

Authorization

Authentication

Injection flaws

Improper error handling/Exception flaws

Encryption (Cryptography)

Auditing and Logging

Session related flaws (Session management)

Insecure configuration

Recommendation: As a security analyst, it’s our duty filter false positives generated by tools and validate them in order to confirm that they are actual flaws. After doing a proper code review, you should document the vulnerabilities found in a comprehensive report coating the category of the vulnerability and their mitigation. You may go one step ahead and suggest a sample secure code. I personally include the code snippets as well in the report, preferably in an excel file and then attach it.

This is how I personally go about it. The points below are based on my experience till date in doing code review. I have still a lot to learn and the points below may or may not hold true in every condition:

Always perform manual reviews: Automated code review is a process where you run the scanning tools like Rational, Ounce Labs, and Parasofton the code base followed by a manual auditing of the results. The scanner flags the whole code with vulnerabilities based on its perception. Now it’s the job of the auditor to differentiate between real issues and false positives. Here is where the real pain starts. You don’t have command over each and every language. So taking help of language specific resources is required. Now, I have got familiar with almost all major languages (.NET, Java, PHP) specific vulnerabilities. Doing a black box assessment you never come to know where the real problem lies.

Talk to developers first: The more you involve developers in your code review process, the more effective the analysis will be. You get confidence that whatever you are doing is based on the right understanding of the code. On the other hand, developers also get happy that you are taking theminto confidence instead of declaring something vulnerable straightaway.

Have a notebook and pen handy to understand the flow of the program. Understanding the source of the taint and where does it reflects in the code is necessary to catch the real vulnerabilities. Just seeing that the taint is entering the program and reflecting in some other part of the program doesn’t always mean that it is vulnerable to Cross Site Scripting, for example. Again here, talking to developers helps, as they might be implementing some centralized input filtering/validation mechanism. So don’t just jump to any conclusions.

Use an advance text editor: The text editor should be capable of searching a term in the whole code base. One such text editor is Notepad++. It searches the term and highlights them so that you can see where all places the particular term is being used. It helps you in joining the pieces and seeing the complete picture.

Have sufficient time to do code review, as you need to apply your thoughts more than once to pick up real vulnerabilities. So always ask your customers for sufficient time in fully completing the project.

Being connected to Internet always helps at the time of code review. Certain terms, functions or methods always annoy you as you might have not have seen them before. Google helps a lot in understanding them.

Find vulnerabilities in context of the application: Not only should you pick up real and applicable vulnerabilities in the context of the application – as it decreases the number of issues – but also, you should propose the countermeasures in the report. That makes developers happy and confident. The scanner may flag any issue as High, Medium or Low. It’s your responsibility to give them appropriate ranking based on applications context.

Everybody loves his own program: Programs are like a developers baby. Don’t always pinpoint the weaknesses of the program, also appreciate them if you find any robust mechanism used in the program. That way, you can make them friendly and they will always come to use to get their code reviewed. So both are then happy.

Train them: Last but not least, train developers about the vulnerabilities in the real world. Give them training, involve them and encourage them to review their codes before production. Tell them how it saves effort and money. If you have a scanning tool that supports plug-ins for IDE, install it at their machines so that they can do proper development and a hand by hand review.

A Sample Illustration:

Consider this example( Owasp WebGoat Project):

String username = “”;

String password = “”;

username = s.getParser().getRawParameter(USERNAME);

password = s.getParser().getRawParameter(PASSWORD);

……………..

………..……

String query = “SELECT * FROM user_system_data WHERE user_name = ‘”

+ username + “‘ and password = ‘” + password + “‘”;

ec.addElement(new StringElement(query));

The inputs from the user are requested through getRawParameter, and assigned to the ‘username’ and ‘password’ variables. Again, they are being used directly in the SQL query without any input validation and also being embedded into the dynamic query. Any malicious user can tamper with this query to run his own arbitrary SQL codes. So if we try to find all the entry points into the codebase (getRawParameter in this case), we may detect injection flaws. Even if we search for SQL queries being used in the code, if we find that they are being used as dynamic queries, they may be a case of a possible SQL injection.

Ninj@S3c is a Security Analyst with a leading MNC. He is predominantly focused on Application Security, Network Security and Wireless Security. Beyond this, he’s interested in Reverse Engineering and Forensics.

I would like to point out, however, that your distinction between “black box” and “white box” testing isn’t entirely accurate.

Black box refers to a testing method where the internal design or structure of the system is not known–you can’t see inside it. (Think of pen testing in this regard.) White box refers to testing where the system design and structure are known to the tester; source code analysis is by definition white box testing.

The difference between automated and manual source code analysis is simply how the white box testing is performed.

– Rob

Ninj@S3c

Hi Rob,
Appreciate your comments on it!

Yes, what you say is correct. In VAPT this holds true. But here my intention was to define black box with Tool based as we really can’t control the execution and but in manual process, ie, white box approach, we really read it manually and look for logical flaws. If misleading will take care, further.
Thanks!

About InfoSec

InfoSec Institute is the best source for high quality information security training. We have been training Information Security and IT Professionals since 1998 with a diverse lineup of relevant training courses. In the past 16 years, over 50,000 individuals have trusted InfoSec Institute for their professional development needs!

Join our newsletter

File download

First Name

Last Name

Work Phone Number

Work Email Address

Job Title

Does your employer pay for training?

What is your timeline for training?

InfoSec institute respects your privacy and will never use your personal information for anything other than to notify you of your requested course pricing. We will never sell your information to third parties. You will not be spammed.

Comments

What is Skillset?

Skillset

Practice tests & assessments.

Practice for certification success with the Skillset library of over 100,000 practice test questions. We analyze your responses and can determine when you are ready to sit for the test. Along your journey to exam readiness, we will:

1. Determine which required skills your knowledge is sufficient
2. Which required skills you need to work on
3. Recommend specific skills to practice on next
4. Track your progress towards a certification exam