Only Software matters

Experiences in software development

Archive for September, 2012

Fixing common Java security code violations in Sonar

Posted by Patroklos Papapetrou on September 21, 2012


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.

Posted in java, quality, software, sonar | Tagged: , , , | 8 Comments »

Resign Patterns – Eliminate them with Agile practices and Quality Metrics

Posted by Patroklos Papapetrou on September 17, 2012


This blog post is inspired by the article titled Resign Patterns by Michael Duell. I’ve included all the original text from the above article but for each anti-pattern I mention (at least) one agile practice that IMHO is helpful eliminating it and one or more quality metrics that would help you identify it very early. 

 

1 Cremational Patterns

Below is a list of five cremational patterns.

1.1 Abject Poverty

The Abject Poverty Pattern is evident in software that is so difficult to test and maintain that doing so results in massive budget overruns.

Agile Practices : Refactoring, TDD
Quality Metrics : LCOM4, RFC, Cyclomatic Complexity

1.2 Blinder

The Blinder Pattern is an expedient solution to a problem without regard for future changes in requirements. It is unclear as to whether the Blinder is named for the blinders worn by the software designer during the coding phase, or the desire to gouge his eyes out during the maintenance phase.

Agile Practices : Simple Design, Program Intently and Expressively
Quality Metrics : LCOM4, Cyclomatic Complexity

1.3 Fallacy Method

The Fallacy method is evident in handling corner cases. The logic looks correct, but if anyone actually bothers to test it, or if a corner case occurs, the Fallacy of the logic will become known.

Agile Practices : Unit Testing, User Stories , Customer Collaboration to define acceptance criteria and precise requirements
Quality Metrics : Code Coverage ( Line + Branch Coverage )

1.4 ProtoTry

The ProtoTry Pattern is a quick and dirty attempt to develop a working model of software. The original intent is to rewrite the ProtoTry, using lessons learned, but schedules never permit. The ProtoTry is also known as legacy code.

Agile Practices : Refactoring, Code in Increments
Quality Metrics : Code Coverage

1.5 Simpleton

The Simpleton Pattern is an extremely complex pattern used for the most trivial of tasks. The Simpleton is an accurate indicator of the skill level of its creator.

Agile Practices : Simple Design, Program Intently and Expressively
Quality Metrics : LCOM4 , Cyclomatic Complexity

2 Destructural Patterns

Below is a list of seven destructural patterns.

2.1 Adopter

The Adopter Pattern provides a home for orphaned functions. The result is a large family of functions that don’t look anything alike, whose only relation to one another is through the Adopter.

Agile Practices : Simple Design, Refactoring, Program Intently and Expressively
Quality Metrics : LCOM4 , Cyclomatic Complexity, RFC 

2.2 Brig

The Brig Pattern is a container class for bad software. Also known as module.

Agile Practices : Refactoring, Code in increment, Write cohesive code
Quality Metrics : Package Complexity, Package Size

2.3 Compromise

The Compromise Pattern is used to balance the forces of schedule vs quality. The result is software of inferior quality that is still late.

Agile Practices : Continuous Integration and Continuous Inspection
Quality Metrics : Technical Debt

2.4 Detonator

The Detonator is extremely common, but often undetected. A common example is the calculations based on a 2 digit year field. This bomb is out there, and waiting to explode!

Agile Practices : Code Reviews, Unit Testing
Quality Metrics : Code Violations 

2.5 Fromage

The Fromage Pattern is often full of holes. Fromage consists of cheesy little software tricks that make portability impossible. The older this pattern gets, the riper it smells.

Agile Practices : Refactoring , Code in Increments
Quality Metrics : Technical Debt

2.6 Flypaper

The Flypaper Pattern is written by one designer and maintained by another. The designer maintaining the Flypaper Pattern finds herself stuck, and will likely perish before getting loose.

Agile Practices : Communicate in Code, Keep a solutions log
Quality Metrics : Documentation Density

2.7 ePoxy

The ePoxy Pattern is evident in tightly coupled software modules. As coupling between modules increases, there appears to be an epoxy bond between them.

Agile Practices : Refactoring, Simple Design, Code in increment
Quality Metrics : Coupling, LCOM4

3 Misbehavioral Patterns

Below is a list of eleven misbehavioral patterns.

3.1 Chain of Possibilities

The Chain of Possibilities Pattern is evident in big, poorly documented modules. Nobody is sure of the full extent of its functionality, but the possibilities seem endless. Also known as Non-Deterministic.

Agile Practices : Communicate in Code, Keep a solutions log
Quality Metrics : Documentation Density

3.2 Commando

The Commando Pattern is used to get in and out quick, and get the job done. This pattern can break any encapsulation to accomplish its mission. It takes no prisoners.

Agile Practices : TDD, Unit Testing, Code in increment, Write Cohesive Code
Quality Metrics : Couplings, Complexity, Code Coverage

3.3 Intersperser

The Intersperser Pattern scatters pieces of functionality throughout a system, making a function impossible to test, modify, or understand.

Agile Practices : Code in increment, Write Cohesive Code
Quality Metrics : Complecity, Couplings

3.4 Instigator

The Instigator Pattern is seemingly benign, but wreaks havoc on other parts of the software system.

Agile Practices : Unit Testing, Continuous Integration
Quality Metrics : Code Coverage, Violations Density

3.5 Momentum

The Momentum Pattern grows exponentially, increasing size, memory requirements, complexity, and processing time.

Agile Practices : Code in increment, Refactoring, Keep It Simple
Quality Metrics : Complexity, Size

3.6 Medicator

The Medicator Pattern is a real time hog that makes the rest of the system appear to be medicated with strong sedatives.

Agile Practices : Continuous Integration
Quality Metrics : Couplings

3.7 Absolver

The Absolver Pattern is evident in problem ridden code developed by former employees. So many historical problems have been traced to this software that current employees can absolve their software of blame by
claiming that the absolver is responsible for any problem reported. Also known as It’s-not-in-my-code.

Agile Practices : Practice Collective Ownership, Attack problems in isolation
Quality Metrics : Unit Testing, TDD

3.8 Stake

The Stake Pattern is evident in problem ridden software written by designers who have since chosen the management ladder. Although fraught with problems, the manager’s stake in this software is too high to allow anyone to rewrite it, as it represents the pinnacle of the manager’s technical achievement.

Agile Practices : Practice Collective Ownership, Be a Mentor
Quality Metrics : 

3.9 Eulogy

The Eulogy Pattern is eventually used on all projects employing the other 22 Resign Patterns. Also known as Post Mortem.

Agile Practices : Continuous Inspection, Code Reviews
Quality Metrics : ALL!!!

3.10 Tempest Method

The Tempest Method is used in the last few days before software delivery. The Tempest Method is characterized by lack of comments, and introduction of several Detonator Patterns.

Agile Practices : Communicate with code, Keep a solutions log
Quality Metrics : Documentation Density

3.11 Visitor From Hell

The Visitor From Hell Pattern is coincident with the absence of run time bounds checking on arrays. Inevitably, at least one control loop per system will have a Visitor From Hell Pattern that will overwrite
critical data.

Agile Practices : Code Reviews, Unit Testing
Quality Metrics : Violations Density

Posted in software | 5 Comments »

 
%d bloggers like this: