1

I have a query string like - 

queryStr.append(" ").append(relation.getJoins()[i].getChildSql()).append(" = :join").append(i);

I am using setParameter to set value of :join but Sonar complains possibility of SQL injection. Is it not allowed to append values in setParameter as below - 

 for (int i = 0; i < parentKeyValues.length; i++) {
                    query.setParameter("join" + i, parentKeyValues[i]);
                }
Knight of the Vale
  • 159
  • 3
  • 11
  • 1
    What does `relation.getJoins()[i].getChildSql()` return? Is **any** of it user-supplied content? (Or does `queryStr` contain any user-supplied content?) – T.J. Crowder Jun 26 '18 at 06:49
  • It returns another query string which does not contain any user supplied content. Neither does queryStr contain any other user-supplied content apart from **:join** – Knight of the Vale Jun 26 '18 at 07:02
  • A more complete example would probably help, but it sounds fine (but I'm not a Sonarcube guy). Sonar is probably just seeing the string manipulation and making a (reasonable) assumption. :-) – T.J. Crowder Jun 26 '18 at 07:03
  • If we let aside sonarqube for a moment then can we say that **"join" + i** in **query.setParameter("join" + i, parentKeyValues[i]);** is SQL injection safe considering there is no other user-supplied parameter ? – Knight of the Vale Jun 26 '18 at 07:06
  • If you're only setting user-supplied content via `setParameter`, then yes, it should be safe. – T.J. Crowder Jun 26 '18 at 07:24

1 Answers1

1

SQL injection is possible in:

.append(relation.getJoins()[i].getChildSql())

only when getChildSql() may return any kind of user provided data.

.append(" = :join").append(i)

only when i is something else than an integer and may contain user provided data.

I don't think that setParameter() could be exploited.

Stefan Steinegger
  • 63,782
  • 15
  • 129
  • 193