SQL injection for Dynamic where conditions in prepared statement

Question

I my application we are collecting some user inputs from UI and based on those values we are generating dynamic SQLs with different 'Where' conditions to query data. It is found that that piece of code has some SQL injection flaw.

I am not able to re arrange this code to prevent that flaq. Any suggestion will be helpfull.

My Application takes four input parameters ,

1. Serial number -This can be, one or none, or two values 2.Created date -This can be, one or none, or two values
2. ReportTypeNumbers- This can be, one or none, or more than two
3. reportTitleNames-This can be, one or none, or more than two

based on these input values, I am constructing Dynamic 'Where' conditions for prepared statement. This SQL has issues. Please help me to rewrite it to fix SQL injection flaw.

Here is the Method which constructs Dynamic SQL.

public void filter(String strSerialNumberLogic, String strSerialNumber1,
        String strSerialNumber2, String strCreationDateLogic,
        long lngCreationDate1, long lngCreationDate2,
        String strTypeNumbers, String strTitles, long lngLoc)
        throws SQLException, ClassNotFoundException {

    StringBuffer strWhere = new StringBuffer();
    List paramList = new ArrayList();
    String arrTypeNumbers[];
    String arrTitles[];
    int i;
    boolean bolHit;

    if (!strTypeNumbers.equals("") || !strTitles.equals("")) {
        arrTypeNumbers = strTypeNumbers.split(",");
        arrTitles = strTitles.split(",");

        bolHit = false;
        strWhere.append("(");

        for (i = 0; i < arrTypeNumbers.length; i++) {
            if (arrTypeNumbers[i].length() > 0) {
                if (bolHit) {
                    strWhere.append(" OR ");
                } else {
                    bolHit = true;
                }

                strWhere.append(" REPORT_NUMBER = ?");
                paramList.add(arrTypeNumbers[i]);
            }
        }

        for (i = 0; i < arrTitles.length; i++) {
            if (arrTitles[i].length() > 0) {
                if (bolHit) {
                    strWhere.append(" OR ");
                } else {
                    bolHit = true;
                }

                strWhere.append(" REPORT_NAME = ?");
                paramList.add(arrTitles[i]);
            }
        }

        strWhere.append(") ");
    }

    if (!strSerialNumber1.equals("")) {

        if (!strWhere.equals("")) {
            strWhere.append(" AND ");
        }
        strWhere.append(" REPORT_FILE_NO " + strSerialNumberLogic + " ? ");
        paramList.add(strSerialNumber1);

        if (strSerialNumberLogic.equals("between")) {
            strWhere.append(" AND ? ");
            paramList.add(strSerialNumber2);
        }
    }

    if (lngCreationDate1 != 0) {


        if (!strWhere.equals("")) {
            strWhere.append(" AND ");
        }

        strWhere.append(" REPORT_CREATION_DATE " + strCreationDateLogic + " ? ");
        paramList.add(Long.toString(lngCreationDate1));

        if (strCreationDateLogic.equals("between")) {
            strWhere.append(" AND ? ");
            paramList.add(Long.toString(lngCreationDate2));
        }
    }

    if (lngLoc != 0) {

        if (!strWhere.equals("")) {
            strWhere.append(" AND ");
        }
        strWhere.append(" REPORT_FILE_LOCATION = ? ");
        paramList.add(Long.toString(lngLoc));
    }
    String finalQuery = "";
    if (!strWhere.equals("")) {
        finalQuery = "WHERE " + strWhere.toString();
    }

    String strSQL = "SELECT * " + "FROM D990800 "
            + "LEFT JOIN D990400 ON REPORT_SYSTEM_ID ||" + " REPORT_NO = REPORT_NUMBER " + finalQuery
            + "ORDER BY REPORT_FILE_NO ASC";


    System.out.println("strSQL:" + strSQL );
    System.out.println("paramList:" + paramList );

    Connection conn = ConnectionFactory.instance().getConnection();
    PreparedStatement preparedStatement = null;
    preparedStatement = conn.prepareStatement(strSQL);

    for (int index = 0; index < paramList.size(); index++) {
        String param = (String) paramList.get(index);

        if (isParsableInt(param)) {
            preparedStatement.setInt(index+1, Integer.parseInt(param));
        } else {
            preparedStatement.setString(index+1, param);
        }
    }

    ResultSet rsReports = preparedStatement.executeQuery();

    buildCollection(rsReports);
    rsReports.close();
    preparedStatement.close();
    conn.close();
}

Answer 1

The way you handle strSerialNumberLogic and strCreationDateLogic does allow for a SQL injection attack. Instead of directly appending their values to the where clause, you should use conditional logic to determine the correct conditional to use:

strWhere.append(" REPORT_FILE_NO ");

switch (strSerialNumberLogic) {
  case "=":
    strWhere.append("= ? ");
    paramList.add(strSerialNumber1);
    break;
  case "!=":
  case "<>":
    strWhere.append("!= ? ");
    paramList.add(strSerialNumber1);
    break;
  case "<":
    strWhere.append("< ? ");
    paramList.add(strSerialNumber1);
    break;
  case "<=":
    strWhere.append("<= ? ");
    paramList.add(strSerialNumber1);
    break;
  case ">":
    strWhere.append("> ? ");
    paramList.add(strSerialNumber1);
    break;
  case ">=":
    strWhere.append(">= ? ");
    paramList.add(strSerialNumber1);
    break;
  case "between":
    strWhere.append("between ? and ? ");
    paramList.add(strSerialNumber1);
    paramList.add(strSerialNumber2);
    break;
  case "not between":
    strWhere.append("not between ? and ? ");
    paramList.add(strSerialNumber1);
    paramList.add(strSerialNumber2);
    break;
  case "is null":
    strWhere.append("is null ");
    break;
  case "is not null":
    strWhere.append("is not null ");
    break;
}

Although you could simply check to ensure that the value of str[SerialNumber|CreationDate]Logic is valid before appending it to avoid injection attacks, your code checker would likely still throw an error, so it's better to append string literals instead of variables.