Inderjeet Singh's Blog

Fortifying Web 2.0 Pet Store

Folks at FortifySoftware are running a program where they run their static analysis tool for code checking and security analysis for free on open-source projects. They were kind enough to run their tool on our Web 2.0 Pet Store application and report bugs to us. In this blog, I share my experiences with some of the subtle errors that their tool caught.

Some of the bugs that fortify reported were spurious because, presumably, the tool doesn't know about Java EE 5 annotations yet. For example, it failed to notice the following dependency injection:

@Resource UserTransaction utx;

The tool reported the above statement as uninitialized variable that may result in a null pointer exception or cause other errors. But since this code belongs to a Java EE managed component, the container will initialize the utx object correctly when the component is instantiated.

The next bug is something we all know academically, but sometimes run into because of oversight. This has to do with integer division when the two operands are roughly of the same magnitude. The result can contain significant value in the fraction, so it is a good idea to store it in a double. So, that is what the petstore code tried to do:

Line 1. int w = image.getWidth();
Line 2. int thumbWidth = thumb.getWidth();
Line 3: double powerW = thumbWidth / w;


However, there is a bug in the above code that fortify caught: In line 3, both thumbWidth and w are int, so the expression thumbWidth /w is an integer division with result converted to an int thereby losing any fraction value that might have been generated. The result is then promoted to a double so that it can be assigned to powerW, but it is a little too late.

The solution, of course, is to cast one of the elements to double before the division so that the operation becomes a double division operation. So, here is the corrected code:

Line 1. int w = image.getWidth();
Line 2. int thumbWidth = thumb.getWidth();
Line 3: double powerW = thumbWidth / (double) w;


Lesson re-learnt.

That brings us to the final type of bug that fortify reported. This relates to cross-side scripting attacks. Essentially, in the petstore we were using some user-submitted data without first validating it. Here is an example:

Line 1:String itemId=request.getParameter("itemId");
Line 2:out.print(itemId);


In this code, we are using the value of the user-submitted paramater itemId without validating it first. A malicious user, can insert escape characters in it that may result in a JavaScript injection attack. Similarly, if this value was being used as a database query parameter, it was amenable to a SQL injection attack. The correct fix is to validate the itemId parameter before using to ensure that it does not contain characters that we do not expect to be there.

Needless to say that we have fixed most of these bugs in the petstore application, and intend to fix the remaining ones soon as well. What are your thoughts on a tool like Fortify? Have you run into similar silly bugs as well? Share your experiences as comments to this blog. Thanks for reading.

Bookmark blog post: del.icio.us Digg DZone Furl Reddit

Comments

Comments are listed in date ascending order (oldest first) | Post Comment

This work is licensed under a Creative Commons License.

Powered by
Movable Type 3.01D