Secure code review: A practical approach

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

Learn Secure Coding

Build your secure coding skills in C/C++, iOS, Java, .NET, Node.js, PHP and other languages.

Learn More

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.

[caption id="" align="alignnone" width="625"] Figure – 1[/caption]

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]:

[caption id="" align="alignnone" width="181"] Figure-2[/caption]

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.

Learn Secure Coding

Build your secure coding skills in C/C++, iOS, Java, .NET, Node.js, PHP and other languages.

Learn More

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.

Sources

• OWASP Code review project
• OWASP code review guide