How to sanitize all variables passed to the selectionArgs array?

Question

Veracode Static Scan report points SQL Injection flaw in my Content Provider implementation.

Previously, I posted this question related to all my doubts regarding this flaw.

And after few discussions I came to a conclusion that there might be chances of it being a false positive in the report. Because according to what I researched and read, I was following the security guidelines mentioned in Android docs and other referenced sources to avoid SQL Injection.

There is suggestion everywhere to perform at least some input validation on the data passed to SQL queries.I want to cover this possibility which be the reason of flaw. Everyone is asking me to sanitize data before passing to query. How do I exactly sanitize variables passed to selectionArgs array passed to delete(), update() method of Content Provider?

Will DatabaseUtils.sqlEscapeString() be sufficient? Please suggest!

Here's the implementation where I need to sanitize the variable:

 public Loader<Cursor> onCreateLoader(int id, Bundle b) {
    switch (id) {
        case THOUGHT_LOADER:
            return new CursorLoader(getActivity(), NewsFeedTable.CONTENT_URI, NewsFeedTable.PROJECTION, NewsFeedTable._id + "=?", new String[]{tid}, null);
        case COMMENT_LOADER:
            return new CursorLoader(getActivity(), CommentTable.CONTENT_URI, CommentTable.PROJECTION, CommentTable.COLUMN_TID + "=?", new String[]{tid}, null);
        default:
            return null;
    }
}

Report points to the flaw :Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection') (CWEID 89) at this line

deleted = db.delete(BulletinTable.TABLE_NAME, selection, selectionArgs); in the below code:

 @Override
public int delete(Uri uri, String selection, String[] selectionArgs) {
    if (uri.equals(Contract.BASE_CONTENT_URI)) {
        deleteDatabase();
        return 1;
    }

    SQLiteDatabase db = openHelper.getWritableDatabase();

    int deleted = 0;
    switch (matcher.match(uri)) {
        case BULLETIN:
            deleted = db.delete(BulletinTable.TABLE_NAME, selection, selectionArgs);
            break;
        case CLASSROOMS:
            deleted = db.delete(ClassroomsTable.TABLE_NAME, selection, selectionArgs);
            break;
        default:
            throw new IllegalArgumentException("Unsupported URI: " + uri);
    }

    if (deleted > 0) {
        getContext().getContentResolver().notifyChange(uri, null);
    }

    return deleted;
}

Answer 1

Values in the selectionArgs array never need to be sanitized, because they cannot be interpreted as SQL commands (that's the whole point of having separate parameter values).

The purpose of sqlEscapeString() is to format a string so that it can be put into an SQL command (i.e., escape single quotes; all other characters have no meaning inside an SQL string). But when you know that there is a string, you should selectionArgs instead, so this function is not helpful.

You need to sanitize only strings that end up in the SQL command itself. In this case, this would be selection. If this value comes from the user, or from some other app, then you have no control over how much stuff your DELETE statement actually does (it could call SQL functions, or execute subqueries that access other parts of the database).

For practical purposes, it is not possible to sanitize strings that are intended to contain SQL commands, because then you would need a full SQL parser. If your content provider is available for external code, you should allow deleting specific items only through the URI, and disallow custom selections.