Mysql – Dynamic column enumeration for preventing SQL injection

MySQLpdoPHPsql

I have an application which is completely for internal use but I'm trying to make sure I have my bases covered on SQL injection. Unless someone else has some information to point me otherwise there is no way to dynamically specify the column listing within a statement without opening the query up to SQL injection as prepared statement bindparam cannot be used to specify a column name.

For example (not possible, would throw errors):

SELECT :column_name FROM tablename WHERE other_column_name = :value

In order to still dynamically allow the calling of the column name I directly have to pass in a variable (works, but is vulnerable to injection):

SELECT $column_name FROM tablename WHERE other_column_name = :value

In order to prevent injection I check the value of $column_name against a whitelist of all the column names within the table using PHP's in_array function. If it's in the list I go ahead with the query but if it's not I throw an exception.

As there are many different places in the application where a query like this is needed I added a function to my PDO wrapper to automatically enumerate these column listings using the data from the information_schema database:

public function schema_list($database_name = NULL, $table_name = NULL, $column_name = NULL)
{   
    $query = 'SELECT table_schema AS database_name, table_name, column_name 
        FROM `information_schema`.`columns` 
        WHERE table_schema = :database_name ';
    $parameters = array();

    // Use this object's database name unless manually specified
    if (!is_null($database_name)) {
        $parameters[':database_name'] = $database_name;
    } else {
        $parameters[':database_name'] = $this->database_detail['dbname'];
    }

    // Add details for table_name if specified
    if (!is_null($table_name)) {
        $query .= 'AND table_name = :table_name ';
        $parameters[':table_name'] = $table_name;
    }

    // Add details for column_name if specified
    if (!is_null($column_name)) {
        $query .= 'AND column_name = :column_name ';
        $parameters[':column_name'] = $column_name;
    }

    $result = $this->query_select($query, $parameters);

    if ($result['sth_count'] == 0) {
        return FALSE;
    } else {
        return $result;
    }
}

Of course this returns a multidimensional array of records, the constructor of the object which eventually needs to enumerate the column listing does:

protected $table_process_columnlist;    // List of valid columns for the process table

public function __construct(dblink $global_dblink = NULL, job $global_job = NULL)
{

// other constructor secret sauce //

// Initialize valid column list
$this->table_process_columnlist = array();
$schema_list = $this->dblink->schema_list(NULL, $this->table_process);
foreach ($schema_list['sth_result'] as $table_column) {
    $this->table_process_columnlist[] = $table_column['column_name'];
}

And is then used by:

public function __get($column)
{
    if (in_array($column, $this->table_process_columnlist)) {
        $query = 'SELECT '.$column.'  
            FROM '.$this->table_process.' 
            WHERE idprocess = :idprocess
            LIMIT 1';
        $parameters = array (
            ':idprocess' => $this->idprocess,
        );

        $result = $this->dblink->query_select($query, $parameters);

        return $result['sth_result'][0][$column];
    } else {
        $e_message = 'Could not get specified column';
        throw new ACException($e_message, 99);
    }
}

Keep in mind I'm not really using prepared statements to gain performance, they are only being used to for SQL injection protection. From what I've found in tests the information_schema table will only return a result set for items which the database handle user actually has access to so the schema_list function couldn't be used to enumerate schema layout of a database which the database user doesn't already have access to.

I'm using a public __get magic function because this class is only used internally in the application and is wrapped by other classes further up in the application.

Is this a sane way to prevent SQL injection while gaining the ability to dynamically specify the column names? Are there any security concerns with this method?

Best Answer

I didn't read your code, but yes, a white list is the correct solution here.