Guarding against SQL Injection

Any developer doing any kind of data insertion of updating within a database needs to understand SQL Injection. From the linked resource:

Someone enters an SQL fragment (the classic example is a drop database statement, although there are many possibilities that don’t include deletions which could be just as destructive) as a value in your URL or web form. Never mind now how he knows what your table names are; that’s another problem entirely. You are dealing with an insidious and resourceful foe.

Well, we know how they might know what the tables are: we're an open source project! That's why you need to guard against SQL injection. The Concrete CMS core is strongly hardened against SQL injection, but any code you write must be as well. Let's take a look at something that's vulnerable. Let's say we're writing some code that wants to print out articles by a user.

<form method="post" action="<?=$view->action('search')?>
    User ID: <input type="text" name="uID" />
    <button type="submit">Search</button>
</form>

Then here's the relevant single page controller method:

public function search()
{
    $db = \Database::connection();
    $ids = $db->GetCol('select cID from Pages where uID = {$this->request->request->get('uID')}');
}

This code may not look like much, but it's vulnerable. What if the user entered something like "1 union select uEmail from Users" ? They might be able to retrieve email addresses or worse.

Solution 1: Always use the API

The first major mistake being made here is the non-usage of the Concrete API. Since the API itself is much less likely than the basics of the SQL schema, it's always a good idea to use the API. Consider this code instead:

public function search()
{
    $ui = \UserInfo::getByID($this->request->request->get('uID'));
    if (is_object($ui)) {
        $list = new \Concrete\Core\Page\PageList();
        $list->filterByUserID($ui->getUserID());
        $results = $list->getResults();
    }
}

Simpler, cleaner, and safe from XSS.

Solution 2: Use Placeholders

If you must use a direct SQL query to access Concrete data, always use placeholders instead of directly passing variables into your query. Placeholders are supported by Doctrine DBAL (our Database Access Layer) and by PHP's PDO, which is what DBAL depends on. More information about Doctrine DBAL, placeholders and security.

Here's how our query would look with placeholder support:

public function search()
{
    $db = \Database::connection();
    $ids = $db->GetCol('select cID from Pages where uID = ?', array($this->request->request->get('uID')));
}

Easy to read, simple, and safe from SQL injection.