Manual Source code Review

Manual Source code Review

[Total: 1    Average: 4/5]



Finding vulnerabilities in any application is a daunting task of a security researchers, these researchers are increasingly engaging with different approach and methodology to hunt down vulnerabilities. Some of the common approach or methodologies are black box testing and white box testing.


Black box testing approach is when researcher doesn’t have any working knowledge and background of the application, they have to do enumeration of technologies, mapping of the application and identification of fault entry points, determining input validation vulnerabilities, or logical security vulnerabilities.


White box testing involve having source code of the application and having proper understanding of the application as well as its purpose, background, environment and framework to best identify key areas of focus.

In software development, a small coding error can result in a critical vulnerability that ends up compromising the security of an entire system or network.


Source Code review is probably the single-most effective technique for identifying security flaws. When used together with automated tools and manual penetration testing, code review can significantly increase the cost effectiveness of an application security verification effort.

This task can be carried out either by some commercial as well as free automated tools or manually review of the code which requires human interaction.


Manual source code review provides insight into the “real risk” associated with insecure code. This is the single most important value from a manual approach. A human reviewer can understand the context for certain coding practices, and make a serious risk estimate that accounts for both the likelihood of attack and the business impact of a breach,

Manual source code review is a time taking process but this lengthy process can be shortened up 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.


Application Details

The next and most important step is to understand how the application is working what are the settings in it. Therefore entry point of code review will be deployment descriptor (web.xml in J2EE, web.config in .NET). In web service testing it’s very important to understand the application, what it is doing, what are the business requirements? Analyze behavior of application. Have an idea of data flow. Explore about application and mine out important details such as APIs, libraries used.


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


Following is the checklist for code review,

  • Authentication & Authorization
  • Cryptography
  • Respecting boundaries (input validation & output encoding)
  • Session management
  • Threat Modeling Terminology
  • Following are the terminology, which a code reviewer should be aware of,
  • Trust zone: User of application
  • Taint: User provided malicious data
  • Taint propagator: The function taking malicious data as input and without validation passing it out
  • Source: The point where malicious input was given e.g. request.getparameter() is source
  • Sink: The place where vulnerability gets executed example the place where xss alerts get reflected

1.Configuration Management

Study the configuration file (deployment descriptor) properly it will depict a lot about deployment. For example you want to see what HTTP methods are used you can see it in web.xml. In WEB-INF→web.xml is present on viewing it we found the setting as shown in Figure 2.

2. Check out Whether Custom Error Pages are Defined:

If the application developer wants, so he can set a custom error page for error code in web.xml as shown in Figure 1.



Figure 1: Use of custom pages for error code 404


Provide a custom error page to avoid leakage of sensitive information.

Disable dangerous HTTP methods. Use Get and Post methods to be on a safer side.

 3. Listing of HTTP Methods Enabled:

Under HTTP-method tag you can see the methods enabled (refer Figure 2)


Figure 2: Use of dangerous HTTP methods

4.  Data Protection in Storage & Transit:

The setting in Figure 3 shows that transport guarantee is none which means that there is no HTTPS setting provided in the application. So, anybody can intercept the communication.


Figure 3: Transport-guarantee not set properly


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


Figure 4: Transport-guarantee setting should be Confidential

5. Hashing of Password:

A weak hashing algorithm was used for password i.e. SHA1. If you see the code as shown in Figure 5 it seems there is encryption of password (Figure 5), but on opening up of definition of encrypt method of crypto class (Figure 6) we can see hashing of password is done by using SHA, but which SHA algorithm is used that is not known.


Figure 5: SafePass variable storing encrypted value of password

If in instance of messagedigest.getinstance SHA is given then it takes SHA1 by default, the same is happening in this case refer Figure 6. SHA1 is a weak hashing algorithm.


Figure 6: SHA for password hash


A strong hashing algorithm should be used to store password in hashed form. There should be a proper salting of hashed data. The salt used should be different for different user. If you are using some encryption algorithm them they should be strong enough. In coming years if computation resources become cheaper then it may be possible to crack the encryption. Therefore for delaying attacker from decrypting your data you can increase number of iteration of encryption algorithm.

6. Hard Coded Password Case.

Sometimes developers put hardcoded password in JSP page as comments. In Figure 7 first comment (red marked as 1) is of JSP another is of HTML (marked as 2). The HTML comment is visible on web page (since it is a JSP page so HTML comments won’t work). Even the JSP comments can be viewed by view source.


Figure 7: Password hard coded as comment


Avoid hardcoding of password in codes.


7. Authorization

The case of CSRF (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 eyes of attacker. An example of it can be depicted in following fig (refer Figure 8).


Figure 8:  ‘User ID’ passed as hidden parameter.

There is a code for updating of accounts as shown in Figure 9.


Figure 9: Code for updating profile

The code in the Figure 9 would update an account on basis of ‘userid’, which is a crucial parameter passed as hidden parameter. This coding flaw can be used by an attacker who would make his own webpage having same action of update as that of application; he will also have victim’s session ID, which would update profile of victim. The cause of this attack is a weak authorization process which is updating data on basis of a parameter without validating whether the parameter is coming from a legitimate user or not.



Use tokens which would help server in identifying its client.

In spring framework 3.0 there has been token remedy for CSRF.


8. Bypassing authentication:

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


Figure 10: Figure 10: User ID getting passed as cookie


Care should be taken that proper session management is followed. The session token should expire after some time. The session token should be random and long sequence, they should be difficult to guess. The cookie attributes should be properly set.


9. Data Validation: Improper Input Validation.

Sometimes many developers use blacklisting technique, which is not a good practice, consider the code displayed in Figure 11. You can see a blacklisting list provided 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.


Figure 11: Use of blacklisting


Why not to use white listing? 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 given by presenter was 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.


10. Logging & Auditing:

Improper Logging Technique: In the below given Figure 12 we can see that the password is logged in as plaintext.


Figure 12: Logging of password in plaintext

Log Obfuscation

Another possible scenario can be:

User ‘Ak’ logged in at 2:46pm

‘Ak’ is coming as a user input. This logging technique can be used as a tool by attacker by giving input as ‘AD logged in at 2:40pm User Ak’ so the logging string will form as –

User ‘AD logged in at 2:40pm User Ak’ logged in at 2:46 pm

This will help to cover the tracks of attacker by forging up the logs.



The logging should not contain elements coming from user as input, as he can obfuscate the logs by giving malicious input in order to mask his activities.

A password should never be logged as plain text.


11. 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 Figure 13.



Figure 13: Creation of a malicious class by attacker

In Figure 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 Figure 14. 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 (Figure 14) and give the malicious class (Figure 13) as input. Thus when the constructor is called, a JDBC connection is made and this will ultimately lead to deletion of table.


Figure 14: Reflection to call the constructor of malicious class


The application should have an authorization check for performing activities such as deletion or manipulation of database. It is suggested that re-authentication should be prompted before performing such action.

12. 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 URL has been taken as input of ‘GET’ request parameter by application (refer highlighted section of Figure 15). Even if a developer used quite a good mechanism to escape XSS types of attacks but he forgot to put a check on URL redirection, in such a case an attacker can give URL of his phishing site as input to get request parameter. The resulting URL will be given to victim thus misguiding a legitimate user.


Figure 15: URL not getting validated


Assume all input is malicious. Use an “accept known good” input validation strategy, i.e., use a whitelist of acceptable inputs that strictly conform to specifications. Use an intermediate disclaimer page that provides the user with a clear warning that they are leaving the current site. Be careful to avoid XSS problems (CWE-79) when generating the disclaimer page. When the set of acceptable objects, such as file names or URLs, is limited or known, create a mapping from a set of fixed input values (such as numeric IDs) to the actual file names or URLs, and reject all other inputs.


13. SMTP Server Spamming Case:

It is worse to provide hardcoded 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 it he can use this vulnerability of victim server to spam some other parties’ mail box here web application hosting server will become a medium of attack.

In the below given figure we can see that mail is getting send 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.


Figure 16:  Using open relay SMTP server


Provide a strong password for SMTP server, which should not be hardcoded.


About The Author








Raashid Shaikh, Sr. Security Analyst at NII Consulting

Leave a Reply

Your email address will not be published. Required fields are marked *