Search code examples
javasql-injectionveracode

SQL injection risk on sort direction?


I am generating a dynamic prepareStatement like the following (fields might change based on the request inputs). Veracode scan keeps on reporting SQL injection risk on - CWE-89: Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection').

Select id from table where (USER_KEY=?) AND (TIMESTAMP between ? and ?) AND (APPLICATION_ID = ? OR APPLICATION_ID = ? )
AND (TITLE = ? ) order by "sortField" "sortDirection" limit "searchlimit"

since everything starting at "order by" cannot be parameterized via question mark symbol ( ? ). I followed an existing article to sanitize the string starting "order by" before concatenate to the prepareStatment. Using prepared statement for Order by to prevent SQL injection java. Sort field and limit checking logic are fine but sort direction checking still triggers SQL injection risk alert.

StringBuilder whereBuilder = new StringBuilder();

...

if(!Utility.checkAllowedSortColumn(Utility.getSQLField(sortField))) {
        throw new ServiceException("Invalid Request: bad SQL sort field resulted from - "+sortField);
    }else {
        whereBuilder.append(" order by ");
        whereBuilder.append(Utility.getSQLField(sortField));
        whereBuilder.append(" ");
    }
    /*      
    if(sortDirection.equalsIgnoreCase("DESC") || sortDirection.equalsIgnoreCase("ASC")){            
        whereBuilder.append(sortDirection);
    }else {
        throw new ServiceException("Invalid Request: bad sort direction - "+sortDirection);
    }*/

    if(Integer.parseInt(searchLimit) < 1){
        throw new ServiceException("Invalid Request: query result limit must be greater than zero");
    }
    else {
        whereBuilder.append(" limit ");
        whereBuilder.append(searchLimit);
    }

    return  whereBuilder.toString();

If I commented out the sortDirection checking logic like above, Veracode scan passed the preparedStatement (of course, it takes DB default ASC). However, I expect sortDirection for DESC or ASC based on the user input. Any suggestion to add sort direction with checking logic without SQL injection risk ?


Solution

  • Your commented block shouldn't introduce any risk to your query. It looks like this Veracode tool is not smart enough to mark it safe. Are you able to cheat it by reorganizing your code like the following?

        if(sortDirection.equalsIgnoreCase("DESC")) {
            whereBuilder.append("DESC");
        } else if (sortDirection.equalsIgnoreCase("ASC")) {
            whereBuilder.append("ASC");
        } else {
            throw new ServiceException("Invalid Request: bad sort direction - "+sortDirection);
        }