Experienced Java/Java EE Engineer (Architect and developer) , team leader and agile enthusiast. Most of the projects where the employee has been involved in the last 10 years were based on J2EE architectures. He has been working as a team leader and project manager for the last 8 years. He was technical leader and member of the development team for major projects in public sector such as National Municipal Registry, Online. Patroklos is a DZone MVB and is not an employee of DZone and has posted 23 posts at DZone. You can read more from them at their website. View Full User Profile

Fixing Common Java Security Code Violations in Sonar

09.25.2012
| 10889 views |
  • submit to reddit
This article aims to show you how to quickly fix the most common java security code violations. It assumes that you are familiar with the concept of code rules and violations and how Sonar reports on them. However, if you haven’t heard these terms before then you might take a look at Sonar Concepts or the forthcoming book about Sonar for a more detailed explanation.

To get an idea, during Sonar analysis, your project is scanned by many tools to ensure that the source code conforms  with the rules you’ve created in your quality profile. Whenever a rule is violated… well a violation is raised. With Sonar you can track these violations with violations drilldown view or in the source code editor. There are hundreds of rules, categorized based on their importance. Ill try, in future posts, to cover as many as I can but for now let’s take a look at some common security rules / violations. There are two pairs of rules (all of them are ranked as critical in Sonar ) we are going to examine right now.

1. Array is Stored Directly ( PMD ) and Method returns internal array ( PMD )

These violations appear in the cases when an internal Array is stored or returned directly from a method. The following example illustrates a simple class that violates these rules.

public class CalendarYear {
 private String[] months;
 public String[] getMonths() {
    return months;    
 }
 public void setMonths(String[] months) {
    this.months = months;
 }
}

To eliminate them you have to clone the Array before storing / returning it as shown in the following class implementation, so noone can modify or get the original data of your class but only a copy of them.

public class CalendarYear {
 private String[] months;
 public String[] getMonths() {
    return months.clone();    
 }
 public void setMonths(String[] months) {
    this.months = months.clone();
 }
}
2. Nonconstant string passed to execute method on an SQL statement (findbugs) and A prepared statement is generated from a nonconstant String (findbugs)

Both rules are related to database access when using JDBC libraries. Generally there are two ways to execute an SQL Commants via JDBC connection : Statement and PreparedStatement. There is a lot of discussion about pros and cons but it’s out of the scope of this post. Let’s see how the first violation is raised based on the following source code snippet.

Statement stmt = conn.createStatement();
String sqlCommand = "Select * FROM customers WHERE name = '" + custName + "'";
stmt.execute(sqlCommand);

You’ve already noticed that the sqlcommand parameter passed to execute method is dynamically created during run-time which is not acceptable by this rule. Similar situations causes the second violation.

String sqlCommand = "insert into customers (id, name)  values (?, ?)";
Statement stmt = conn.prepareStatement(sqlCommand);

You can overcome this problems with three different ways. You can either use StringBuilder or String.format method to create the values of the string variables. If applicable you can define the SQL Commands as Constant in class declaration, but it’s only for the case where the SQL command is not required to be changed in runtime. Let’s re-write the first code snippet using StringBuilder

Statement stmt = conn.createStatement();
stmt.execute(new StringBuilder("Select FROM customers WHERE name = '").
                         append(custName).
                         append("'").toString());

and using String.format

Statement stmt = conn.createStatement();
String sqlCommand = String.format("Select * from customers where name = '%s'", custName);
stmt.execute(sqlCommand);

For the second example you can just declare the sqlCommand as following

private static final SQLCOMMAND = insert into customers (id, name)  values (?, ?)";

There are more security rules such as the blocker Hardcoded constant database password but I assume that nobody is still hardcodes passwords in source code files…

In following articles I’m going to show you how to adhere to performance and bad practice rules. Until then I’m waiting for your comments or suggestions.

Published at DZone with permission of Patroklos Papapetrou, author and DZone MVB. (source)

(Note: Opinions expressed in this article and its replies are the opinions of their respective authors and not those of DZone, Inc.)

Comments

Robert Saulnier replied on Wed, 2012/09/26 - 7:44am

You didn't fix the security problem, you hid it. This is a classic SQL injection problem. Using StringBuilder or String.format won't fix the problem.

Patroklos Papapetrou replied on Thu, 2012/09/27 - 2:06am in response to: Robert Saulnier

Agree. The best solution is to use PreparedStatement

Comment viewing options

Select your preferred way to display the comments and click "Save settings" to activate your changes.