Improve taint analysis to avoid SQL Injection detected when StringBuilder is used

[domak]

Hi,

As soon as I see a StringBuilder in JPA EntityManager.createQuery(), an SQL injection is detected:

StringBuilder buff = new StringBuilder("from Tiers t");
// add conditional where clauses

TypedQuery<Tiers> query1 = em.createQuery(buff.toString(), Tiers.class); // SQL injection detected
query1.getResultList();

Is it possible to improve detector by checking that no input parameter (method parameter or attribute) has been added to the StringBuilder?

On the code I try to verify, there are a lot of where clauses added if a parameter is not null (and the developper uses setParameter() method)

[h3xstream]

For the moment, the detection of injection was base on code from Findbugs core plugin.
The next thing I'll be working on is improving(/rebuild) the detection of constant string. (including String builder using only constants)
I will include a equivalent sample in the test cases.

[h3xstream]

The initial code sample with StringBuilder is now supported. Even much more advanced flow analysis are done in 1.4.2.
The release version will be available very soon.

[ThrawnCA]

I'm still getting false positives from SQL_INJECTION_HIBERNATE when assembling a constant string:

private static final String NTP_QUERY = new StringBuilder(
                " select os.name, p.dateProcessed, cd.title || ' ' || cd.givenName || ' ' || cd.familyName, p.amount ")
        .append("   from Statement as st ")
        .append("  inner join st.paymentRequest as pr ")
        .append("  inner join pr.onlineService as os ")
        .append("   left outer join pr.contactDetails as cd ")
        .append("  inner join st.payment as p ")
        .append("  where p.dateProcessed >= :startDate ")
        .append("    and p.dateProcessed <= :endDate")
        .append("  order by p.id")
        .toString();

and then passing it to Hibernate Session createQuery.

[h3xstream]

Field are different beast.

Unless the type is primitive final instance can be altered. Here it is an immutable type String with a twist. In general, we didn't need to worry about constant String because the compiler does optimization were it will copy the content of constant everywhere they are used (inline into the code of methods). This late initialization does not allow the compiler to optimize the use of the string.

I think it is worth to be supported.

[h3xstream]

@ThrawnCA I have created #385