Secure Code Review for Beginners

Its an excerpt from Null Bachaav session held in July,2014, Special thanks to Sandesh Anand for conducting this outstanding session.

Manual Code Review is a time taking process but this lengthy process can be shortened if we know properly, which areas to investigate. Although many tools are available but still proper knowledge of manual code review is required. The first and foremost requirement of Code Review is that you should have basic understanding of at least one Object Oriented Framework i.e. J2EE, .net. Knowledge of PHP is also fine.

Understanding the Application

The most important step in code review is to understand the context of the application plus what are the settings configured for the application. To perform code review of an application it is important to treat deployment descriptor (web.xml in J2EE, web.config in .net) as an entry point. 

In web service testing it’s very important to understand the application, what is the purpose of the application, what are the business requirements.It is recommended to analyze behavior of the application, have an idea of data flow ,explore the application and mine out important details such as APIs, framework and libraries used.

Note: In PHP there is no configuration file present if no framework is used

Checklist of Code Review

The checklist of code review is quite enormous but as a beginner you can try to investigate these points:

• Authentication & Authorization
• Cryptography
• Respecting boundaries (input validation & output encoding)
• Session management
• Configuration of Application
• Logging

Introducing Few Jargons of Code Review

1. Trust Zone: User of the application.
2. Taint: Malicious data provided by the user.
3. Taint Propagator: The function taking malicious data as input and passes it to destination with proper validation.
4. Source: the point where input was given, eg request.getparameter() is source.
5. Sink: the place where vulnerability gets executed, eg the place where xss alerts get reflected.

These terminology will be helpful in developing a well defined threat model.

Fundamentals of Code Review

Configuration Management

The configuration of application(deployment descriptor) should be properly analyzed as they depict a lot about the application and its deployment.

• Listing of Enabled HTTP Methods 

The web.xml can be viewed to find out enabled HTTP methods. Under HTTP-method tag you can see enabled methods(refer Fig 1).In this case we can see that dangerous methods such as PUT/DELETE/TRACE are allowed.

Fig 1: Dangerous HTTP Methods Enabled 

Dangerous HTTP methods should be disabled in configuration file.

• Check whether Custom Error Pages are Defined

In web.xml file search for error-pages tag, there you can check whether error page is defined or not. If a custom error page is not defined then it may be a possibility that the error messages sent by server may contain server related sensitive information.

A developer can set a custom error page in web.xml as shown in Fig 2.

Fig 2: Use of Custom Pages for Error Code 404

Data Protection in Storage & Transit

• HTTPS Not Used

Consider Fig 3, the setting shows that the transport guarantee is none which means that there is no HTTPS setting, therefore anybody can intercept the communication. 

Fig 3: HTTPS is not configured

The setting for transport guarantee should be confidential as shown in Fig 4.

Fig 4: transport-guarantee should be set as confidential in order to enable HTTPS for a site

• Hashing of Password

If you refer the code as shown in Fig 5 it seems there is some encryption done on password but on opening the definition of crypto class' encrypt method (Fig 6) we can see hashing of password is done by using SHA, but which version of SHA algorithm is not known. If parameter of messagedigest.getinstance contains SHA then it takes SHA1 by default, the same is happening in this case and we all know that SHA1 is a weak hashing algorithm. 

Fig 5: Code shows some encryption done on the password

Fig 6: SHA is used to perform hashing of password

A strong hashing algorithm should be used to compute hash of sensitive data. There should be proper salting of hashed data and the salt used should be different for different user. If you are using some encryption algorithm then they should be strong enough as in coming years if computational resources become cheaper, it may be possible to crack the encryption. Therefore, for delaying the attacker from decrypting the data it will be good if you increase number of iterations in encryption algorithm.

Hard Coded Passwords

Sometimes developers put hard coded password in the code as comments which is not  a good practice. If you consider Fig7 a developer leaves few comments in the code, the first comment is of jsp and another is of html. If an attacker is accessing the webpage, he views the page source, he will be able to see the hard coded password in html comments!!!

Fig 7: Hard coded password in comments

Authorization

• Bypassing Authorization

“My profile is better than you because I have updated it”

Sometimes developers have a wrong notion that sending parameters/session token as a hidden field or obscuring with programming terminologies would save them from attacker. An example of it can be depicted in Fig 8.

Fig 8: userid  passed as hidden parameter 

When we consider the code used for updating a profile(Fig 9), we see that on the basis of userid a profile is getting updated. This makes userid a crucial parameter...wait!!! ...this crucial parameter is passed as a hidden parameter. An attacker can make use of this coding flaw by simply intercepting update profile request and modifying the userid to victim's userid. 

Fig 9: Code for updating a profile

• Bypassing Authorization (case of CSRF)

In case, a server performs profile update by validating userid with session id then also an attacker can bypass authorization by CSRF. He can craft a webpage which performs the similar action as that of website and lure a logged in victim to visit his page. When victim visits the page his browser sends the session id along with the request thus nullifying the above authorization validation technique.

• Cause of Authentication Bypass

The cause of this attack is a weak authorization process which is updating data on the basis of a parameter without validating whether the parameter is coming from a legitimate user or not.

It can be avoided by using random tokens which can help server in identifying its client. In spring framework 3.0 there is a token remedy for CSRF. You can use various frameworks such as ESAPI to prevent such attacks.

Authentication

• Bypassing Authentication

In Fig 10 you can see the session token’s value is stored in a hidden field, which can be easily retrieved by MITM plus it is simply userid. An attacker can use session token of victim to bypass authentication process and gain an access to the victim’s profile.

Fig 10: 'userid' getting passed as session id

Care should be taken that proper session management is followed. The session token should not be guessable. It should be long, random and should expire after some time. The cookie attributes should be set properly.

Data Validation

Sometimes many developers use blacklisting technique, which is not a good practice, consider the code displayed in Fig 11. You can see a blacklisting methodology implemented by developer. He is blacklisting script pattern, javascript pattern, eval, onload but he has not included onmouseover and many other attack vectors in the list.

Fig 11: Use of blacklisting

Why not to use whitelisting? If I am providing validation of phone number textbox then why should I say don’t allow alphabets, special characters instead I can say allow only numbers. Isn’t it simple? Just tell what is valid.

Another recommendation is to remove the burden of validation from developer and put it on secure Frameworks such as Spring or some secure APIs. Nowadays there are various APIs and framework available to do the job of input validation and output encoding.

Logging & Auditing

• Improper Logging Technique(plain text password in logs)

In below given Fig12 we can see that the password is logged in plain text

Fig 12: Password logged in clear text

• Log Forging

Considering a possible scenario:

User ”Ak” logged in at 2:46pm  - in this case "Ak" is user input. What if attacker gives ”AD logged in at 2:40pm User Ak”  as input ? The log entry will now be User “AD logged in at 2:40pm User Ak” logged in at 2:46 pm. The forging of logs will help an attacker to cover his tracks.

Improper Use of Reflections

• What is reflection?

Reflection is the ability to examine or modify the properties or behavior of an object at runtime.

• Scenario of attack

I am a disgruntled employee, I am resigning, but I have been so much frustrated with my job that I have an intention to harm company without getting caught by logging of application. I create the page as shown in Fig 13.

Fig 13: the constructor of ReflectionandMore contains code of deleting  a database table

In Fig 13 you can see that a class ReflectionandMore is created which has a constructor defined. In the definition of constructor, a connection to database is formed and the next step is deletion of some table. This class is not called anywhere in application, therefore when you run code review tool on it, the tool will not consider it as malicious code and leave it marked as a dead code.

Here comes the attack by using reflection as a medium, refer Fig14. In the figure you can see that the reflection is creating instance of class which is provided as user input 's'. So attacker will call this code (Fig 14) and give the malicious class (Fig 13) as input. Thus during dynamic creation of object (ReflectionandMore object) the constructor is called, a JDBC connection is made and this will ultimately lead to deletion of table ‘name’.

Fig 14: Using reflection to create instance of malicious class 

URL Redirection Case

There are cases when developers make use of a good output encoding mechanism that is c:out to escape injections but sometimes some remedies are applicable for some attacks only. In this case (Fig 15) URL has been taken as input by application. Developer used quite a good mechanism to escape XSS types of attacks but he forgot to put a check on URL redirection. An attacker can give URL of his phishing site as input thus misguiding a legitimate user.

Fig 15: URL not getting validated

SMTP Server Spamming Case

It is worse to provide hard coded password of SMTP in code but it is worst case to have no password defined for SMTP server. If anyhow attacker comes to know of the flaw, he can spam some other parties’ mail box by using the vulnerable server as medium of attack. 

In the Fig 16 we can see that mail is getting sent by using open relay SMTP server. Open relay allows anyone on internet to send mail. They are really unpopular as they can be exploited by spamming.

Fig 16: Open Relay SMTP server used

These were few cases discussed by the presenter to provide us a basic idea of code review, however lot of hard work, practice and thirst for knowledge is required to become proficient in code review.