SQL injection holes everywhere... The original author of the product put concatenated SQL queries throughout the whole application. If it's not the client asked for a penetration test, we as developers wouldn't even be given chance to fix this shit.

I'm actually glad to have the chance. I can't live seeing them every day but force myself to ignore them.

And it is actually so easy to do.

Either use real sql parameters or what is the corresponding type or wrap every argument you need to concat in a method call.

One for strings that replace every ' with '' (2 ticks).

One for numbers that verify that it is a number and convert it to string of the correct format.

And so on for dates an booleans.

And make sure to only use them when you concatenate, do not store converted values in variables, some one will copy the concatenation but not the call that saves the secure value.

This practice has kept sql injections at bay for 16 years now :)

@Voxera well said. Though these days almost all languages have some mechanism to avoid sql injection.

So they meet the ol' Bobby tables?

@yendenikhil yes. But for some very dynamic questions they can be difficult to use.

We have some that are constructed from over a hundred fragments depending on what columns are needed.

Also, in some rare occasions with many parameters we have found that the query planner goes bonkers and by concatenating some arguments with a very unbalanced cardinality we can get it to pick the right query plan for both alternatives.

With a parameter it tries to create a plan that work for both 100 rows and for a million.

And with a table of 1.5 billion rows and a dozen sen joins it really makes a huge difference.

We are talking minutes vs milliseconds depending on plan :/

@Voxera I am not talking hibernate like structure where the sql and totally abstracted and tuning is difficult. You can simply parse the query with proper whitelisting conditions (which you don't have build).

Having said that I concede there could be some scenarios where it is not feasible.

The changes are very easy. Easier than what have been discussed here. We're using Java so it's a plain conversion of Statement to PreparedStatement.

The difficult part or what's been stopping us from doing it is the fear that "making changes can create more bugs"... Not until recently we finally have come up with some change management/control hopefully can stop people from shooting at other's foot.

@lwhken I know. We have been running with the methods since 2000 and only now sql parameters are nearing a majority of calls.

A lot of "if it works dont fix".

@Voxera unfortunately I think it's a bit older than 2000... 1997 I reckon :/