CodeSOD: Constantly Querying

Arguably, the biggest problem with SQL as a query language is that we usually execute SQL statements from inside of some other programming language. It tempts us into finding quick hacks to generate dynamic SQL statements, and if we do it the quick way, we find ourselves doing a lot of string concatenation. That way lies SQL injection vulnerabilities.

Constructing SQL statements by stringing together text is always a bad idea, even if you're still using query parameters. There's a reason why most modern database wrappers provide some sort of builder pattern to safely construct dynamic queries. Even so, everyone wants to find their own… special way to accomplish this.

Take this sample from Sciros:

// Submitter's note: redacted or modified domain or sensitive data (it's not originally about ordering pizza)

package com.REDACTED;

/**
 * Stores skeletal SQL statments associated with the pizza order domain.
 * 
 * @author <a href="mailto:REDACTED@REDACTED.com">REDACTED</a>
 */
public interface PizzaOrderSQL
{
    /**
     * The SQL keyword to order results ascending.
     */
    public static final String ASC_SQL_KEYWORD = 
        "ASC";

    /**
     * The SQL keyword to order results descending.
     */
    public static final String DESC_SQL_KEYWORD = 
        "DESC";

    /**
     * The common FROM SQL clause that represents the count of records. 
     */
    public static final String RECORD_COUNT_FROM_CLAUSE = 
        "SELECT     COUNT(*) ";

    /**
     * The SQL clause to constrain pizza orders to exclude certain toppings.
     */
    public static final String EXCLUDED_TOPPINGS_SQL_CLAUSE = 
        "           AND "                                                               +
        "           B.TOPPING NOT IN (%s) " /* excludes test toppings */;
    
    /**
     * The SQL clause to constrain pizza orders by status type.
     */
    public static final String PO_BY_STATUS_SQL_CLAUSE = 
        "           AND "                                                   +
        "           A.STATUS_CODE IN (%s)";

    /**
     * The SQL clause to constrain pizza orders that start at or earlier than a certain time
     */
    public static final String PO_BY_START_TIME_AT_OR_BEFORE_SQL_CLAUSE = 
        "           AND "                                                   +
        "           A.START_TIME <= TO_DATE('%s', 'MM/DD/YYYY HH24:MI:SS')";

    /**
     * The SQL clause to order returned pizza orders by delivery vehicle.
     */
    public static final String WO_ORDER_SQL_CLAUSE = 
        "ORDER      BY B.DELIVERY_VEHICLE_ID";

    //... REDACTED lots of massive select and where clauses that follow the gist of those shown.
}

ASC_SQL_KEYWORD is automatically funny, given that it's vastly longer than the actual keyword. But RECORD_COUNT_FROM_CLAUSE is pretty amazing, since that's a SELECT clause, not a FROM clause.

This actually gets used with string concatenation to build the queries:

string query = RECORD_COUNT_FROM_CLAUSE + PIZZA_ORDERS_TABLE_CLAUSE + PO_BY_START_TIME_AT_OR_BEFORE_SQL_CLAUSE + EXCLUDED_TOPPINGS_SQL_CLAUSE +
    ORDER_BY_DATE_CLAUSE + DESC_SQL_KEYWORD;

It does at least use parameters. Now, whether those parameters are actual query parameters or just printf-style string-munging, we'll… I'll give them the benefit of the doubt, because it's bad enough as it is.

[Advertisement] Utilize BuildMaster to release your software with confidence, at the pace your business demands. Download today!

This post originally appeared on The Daily WTF.