Avoid SQL Injection when using Dynamic SQL Code

Question

I am working on an security remediation of an existing java web application. The application has some dynamic sql code executed by JDBC.But, this is not accepted by Static Code analysis tool we use. So, I am looking for a way to remediate the issue.Basically, I have validated all the input passed to code which constructs the query , so there is no possiblity of SQL Injection. But, the SCA tool still does not approve of this validation. So, want to know if there is any way I can avoid Dynamic Query logic. Prepared Statements cannot be used as the query is dynamicly constructed based on conditions.

I know Stored Procedure can help. But, I understand it has its own issues and the team is also not experienced on Stored Procedures. So, looking for a better way to address this issue. Also, since we are using SQL Server I didn't find any encoding function in the ESAPI toolkit to sanitize the query parameters which has support for oracle and mysql only.

Want to know if using a framework like Mybatis to offload the java code which constructs sql to xml files would resolve the issue. Can you guys let me know if there is any other better way.

Answer 1

I've found this question while trying to solve similar problem myself.

First, we may factor sql code out of java files and store it in text file under resources folder. Then, from java code, use classloader's method to read sql as inputStream and convert it to String. Storing sql code in separate files will enable statical code analysis.

Second, we can use named parameters in sql in some form that is easily recognizable via regular expressions. E.g. ${namedParam} syntax which is familiar by different expression languages. Then we can write helper method to take this parametrised sql and Map<String, Object> with query params. Keys in this map should correspond to sql parameter names. This helper method would produce PreparedStatement with set parameters. Using named parameters will make sql code more readable and will save us some debugging.

Third, at last, we can use sql comments to mark parts of sql code as dependant on presence of some parameter. And use it in the previously described helper method to include in the resulting Statement only parts, for which entries in parameters Map exist. E.g.: /*${namedParam}[*/ some sql code /*]${namedParam}*/. This would be an unobtrusive way to insert conditions into our dynamic sql.

Following DRY principle, we could also try to employ some existing expression language engine, but it would get us one more dependency and processing expense.

I will post the solution here once I get working code.

Answer 2

You can generate SQL dynamically and use prepared statements.

Here is the idea how this can be done. Now you have code like this:

StringBuilder whereClause = new StringBuilder();


if (name != null) {
    whereClause.append(String.format("name = '%s'", name));
}

// other similar conditions

String sql = "select * from table" + (whereClause.length() != 0 ? "where " + whereClause.toString() :  "");

Statement stmt = connection.createStatement();

ResultSet rs = stmt.executeQuery(sql);

// use rs to fetch data

And you need to change this to something like

StringBuilder whereClause = new StringBuilder();
ArrayList<Object> parameters = new ArrayList<>();

if (name != null) {
    whereClause.append("name = ?");
    parameters.add(name);
}

// other similar conditions

String sql = "select * from table" + (whereClause.length() != 0 ? "where " + whereClause.toString() :  "");

PreparedStatement stmt = connection.prepareStatement();

for (int i = 0; i < parameters.length(); ++i) {
    setParameterValue(stmt, i + 1, parameter.get(i));
}

ResultSet rs = stmt.executeQuery(sql);

// use rs to fetch data

setParameterValue should look like this:

void setParameterValue(PreparedStatement ps, int index, Object value) {
    if (value instanceof String) {
        ps.setString(index, (String)value);
    } if (value instanceof Integer) {
        ps.setInt(index, (Integer)value);
    } // and more boilerplate code like this for all types you need
}

With mybatis you can avoid writing such boilerplate code do generate dynamic sql and make this much easier. But I don't know how CSA treats mybatis generated SQL.