Hibernate SQL Injection - Fortify 360

Question

I am using Fortify 360 to analyze my code's security. Fortify has identified a Hibernate based delete method to perform a delete based on an object that has been passed in. My understanding of hibernate is that hibernate will delete a row in the table the object is associated with based on the objects id (the id is a number). Fortify has flagged this as SQL Injection:Hibernate(Input Validation and Representation, Data Flow). Fortify does not seem to like that Hibernate is executing a dynamic SQL statement using unvalidated input. Would a certain fix be required or just caveat this as a mitigated risk?

public List findByItemProp(String propName, Object value){
   try{
     String qs = "from ItemTypes as model where model."+ propName +"= ?"; /* <-- flagged by Fortify */
     Query qo = getSession().createQuery(qs);
     qo.setParameter(0, value);
     return qo.list();
   } catch (){}
 }

score 7 · Answer 1 · edited May 23 '17 at 11:33

I'm afraid this report is probably too lacking in details to provide a diagnosis. I can't diagnose this issue for you without seeing the code.

You definitely should not disregard a static analysis warning as "mitigated risk" when you haven't done anything to mitigate it and don't understand whether the warning represents an exploitable vulnerability or not.

I suggest you start by educating yourself about SQL injection, about security Hibernate, and about this particular warning. Most likely Fortify has online help that provides a detailed explanation about this warning; you should start by reading that. Read the following Fortify backgrounder on SQL injection in Hibernate. I also recommend the following articles: How is SQL injection typically stopped in a Spring/Hibernate setup, ORM injection, and How to avoid SQL injection in Hibernate (A Hibernate Urban Legend).

Update (4/20): I see that you updated the question to include the code. Thanks. The code you provided is indeed dubious, and I think it is reasonable for Fortify to complain about that code. Looking at this code, it is not possible to verify that the query passed to createQuery() is a compile-time constant, which is why Fortify issues a warning message.

Basically, the code you show appears to be constructing a SQL query using string concatenation. That's dangerous. If propName can be influenced by the attacker, then you've got yourself a possible SQL injection vulnerability. It is best to avoid constructing the SQL query in this way.

In particular, there is a myth out there that using prepared statements (or parametrized queries) is inherently immune from SQL injection. This is true, but only if the query itself is a compile-time constant that cannot be influenced by the attacker. If you insert untrusted data into the query itself, then using it in a prepared statement doesn't somehow magically make you safe.

So the best practice is that, when you use a prepared statement, make sure the query can be readily verified to be a compile-time constant (and cannot be influenced by the attacker).

To sum up the general rules of thumb: (1) Don't use string concatenation to build up a SQL query. (2) When you want some dependence upon runtime values, use prepared statements to bind runtime values. (3) Make sure the query that you use to create the prepared statement can be readily verified to be a compile-time constant. Following these practices should help ward off SQL injection attacks.

score 2 · Answer 2 · edited Mar 17 '17 at 13:14

I got the answer from the article “How to avoid SQL injection in Hibernate” that D.W. cited.

Fortify was identifying the the query string qs as vulnerable to SQL injection. I did not understand how SQL injection could occur since I am using setParameter() like the Hibernate article advised. Finally I realized it seemed Fortify seemed to think propName was user-supplied data. When I made the following change

  String qs = "from ItemTypes as model where model.stapler = ?";

Fortify stopped identifying this code as an SQL injection vulnerability.

score 1 · Answer 3 · answered Apr 20 '12 at 21:21

"from ItemTypes as model where model."+ propName

findByItemProp("stapler = '';drop table ItemTypes;--", "");

Oops, there goes your database, yeah it's detecting the string concatenation, don't do that. It's probably not smart enough to realize it's a parameter on a method call, but it's still kind of sloppy being as a programmer wouldn't expect to know that they have to sanitize that without proper documentation, and it's better to make sure it's clean in the method itself being as you'll spare yourself infinite headaches when another developer does it wrong.

Hibernate SQL Injection - Fortify 360

3 Answers3