Fix code scanning alert - Query built from user-controlled sources

[sw23]

It'd be good to switch this over using the recommendation that the tool provides (using PreparedStatement)

Tracking issue for:

• [ ] https://github.com/anwilli5/coin-collection-android-US/security/code-scanning/4

[SanjaySargam]

The link u provided is invalid

[sw23]

@SanjaySargam Looks like the security alerts are not accessible by default. Here's what needs to change: https://github.com/anwilli5/coin-collection-android-US/blob/master/app/src/main/java/com/coincollection/DatabaseAdapter.java#L210-L229

Recommendation Usually, it is better to use a SQL prepared statement than to build a complete SQL query with string concatenation. A prepared statement can include a wildcard, written as a question mark (?), for each part of the SQL query that is expected to be filled in by a different value each time it is run. When the query is later executed, a value must be supplied for each wildcard in the query.

In the Java Persistence Query Language, it is better to use queries with parameters than to build a complete query with string concatenation. A Java Persistence query can include a parameter placeholder for each part of the query that is expected to be filled in by a different value when run. A parameter placeholder may be indicated by a colon (:) followed by a parameter name, or by a question mark (?) followed by an integer position. When the query is later executed, a value must be supplied for each parameter in the query, using the setParameter method. Specifying the query using the @NamedQuery annotation introduces an additional level of safety: the query must be a constant string literal, preventing construction by string concatenation, and the only way to fill in values for parts of the query is by setting positional parameters.

It is good practice to use prepared statements (in SQL) or query parameters (in the Java Persistence Query Language) for supplying parameter values to a query, whether or not any of the parameters are directly traceable to user input. Doing so avoids any need to worry about quoting and escaping.

Example In the following example, the code runs a simple SQL query in two different ways.

The first way involves building a query, query1, by concatenating an environment variable with some string literals. The environment variable can include special characters, so this code allows for SQL injection attacks.

The second way, which shows good practice, involves building a query, query2, with a single string literal that includes a wildcard (?). The wildcard is then given a value by calling setString. This version is immune to injection attacks, because any special characters in the environment variable are not given any special treatment.

{
    // BAD: the category might have SQL special characters in it
    String category = System.getenv("ITEM_CATEGORY");
    Statement statement = connection.createStatement();
    String query1 = "SELECT ITEM,PRICE FROM PRODUCT WHERE ITEM_CATEGORY='"
        + category + "' ORDER BY PRICE";
    ResultSet results = statement.executeQuery(query1);
}

{
    // GOOD: use a prepared query
    String category = System.getenv("ITEM_CATEGORY");
    String query2 = "SELECT ITEM,PRICE FROM PRODUCT WHERE ITEM_CATEGORY=? ORDER BY PRICE";
    PreparedStatement statement = connection.prepareStatement(query2);
    statement.setString(1, category);
    ResultSet results = statement.executeQuery();
}