[BUGFIX] SQL Injection vulnerability in prepared statements
authorHelmut Hummel <helmut.hummel@typo3.org>
Tue, 13 Sep 2011 18:32:19 +0000 (20:32 +0200)
committerOliver Hader <oliver@typo3.org>
Wed, 14 Sep 2011 09:30:44 +0000 (11:30 +0200)
Change-Id: Iaa91761cd2dafbe7be1d6f6c5db169e3a8f48b0c
Resolves: #29400
Releases: 4.6, 4.5
Reviewed-on: http://review.typo3.org/4961
Reviewed-by: Oliver Hader
Tested-by: Oliver Hader
t3lib/db/class.t3lib_db_preparedstatement.php
tests/t3lib/db/class.t3lib_db_preparedstatementTest.php

index ae3e7ab..24a8f22 100644 (file)
@@ -127,6 +127,13 @@ class t3lib_db_PreparedStatement {
         */
        protected $resource;
 
+       /**
+        * Random token which is wrapped around the markers
+        * that will be replaced by user input.
+        * @var string
+        */
+       protected $parameterWrapToken;
+
        /**
         * Creates a new PreparedStatement. Either $query or $queryComponents
         * should be used. Typically $query will be used by native MySQL TYPO3_DB
@@ -146,6 +153,7 @@ class t3lib_db_PreparedStatement {
                $this->table = $table;
                $this->parameters = array();
                $this->resource = NULL;
+               $this->parameterWrapToken = $this->generateParameterWrapToken();
        }
 
        /**
@@ -430,6 +438,11 @@ class t3lib_db_PreparedStatement {
         * @return void
         */
        protected function replaceValuesInQuery(&$query, array &$precompiledQueryParts, array $parameterValues) {
+
+               if (count($precompiledQueryParts['queryParts']) === 0) {
+                       $query = $this->tokenizeQueryParameterMarkers($query, $parameterValues);
+               }
+
                foreach ($parameterValues as $key => $typeValue) {
                        switch ($typeValue['type']) {
                                case self::PARAM_NULL:
@@ -455,26 +468,57 @@ class t3lib_db_PreparedStatement {
                                if (count($precompiledQueryParts['queryParts']) > 0) {
                                        $precompiledQueryParts['queryParts'][2 * $key + 1] = $value;
                                } else {
-                                       $parts = explode('?', $query, 2);
+                                       $parts = explode($this->parameterWrapToken . '?' . $this->parameterWrapToken, $query, 2);
                                        $parts[0] .= $value;
                                        $query = implode('', $parts);
                                }
                        } else {
-                               if (!preg_match('/^:[\w]+$/', $key)) {
-                                       throw new InvalidArgumentException('Parameter names must start with ":" followed by an arbitrary number of alphanumerical characters.', 1282348825);
-                               }
-
                                for ($i = 1; $i < count($precompiledQueryParts['queryParts']); $i++) {
                                        if ($precompiledQueryParts['queryParts'][$i] === $key) {
                                                $precompiledQueryParts['queryParts'][$i] = $value;
                                        }
                                }
-                                       // Replace the marker (not preceeded by a word character or a ':' but
-                                       // followed by a word boundary)
-                               $query = preg_replace('/(?<![\w:])' . $key . '\b/', $value, $query);
+                               $query = str_replace($this->parameterWrapToken . $key . $this->parameterWrapToken, $value, $query);
                        }
                }
        }
+
+       /**
+        * Replace the markers with unpredictable token markers.
+        *
+        * @throws InvalidArgumentException
+        * @param string $query
+        * @param array $parameterValues
+        * @return string
+        */
+       protected function tokenizeQueryParameterMarkers($query, array $parameterValues) {
+               $unnamedParameterCount = 0;
+               foreach ($parameterValues as $key => $typeValue) {
+                       if (!is_int($key)) {
+                               if (!preg_match('/^:[\w]+$/', $key)) {
+                                       throw new InvalidArgumentException('Parameter names must start with ":" followed by an arbitrary number of alphanumerical characters.', 1282348825);
+                               }
+                               // Replace the marker (not preceeded by a word character or a ':' but
+                               // followed by a word boundary)
+                               $query = preg_replace('/(?<![\w:])' . $key . '\b/', $this->parameterWrapToken . $key . $this->parameterWrapToken, $query);
+                       } else {
+                               $unnamedParameterCount++;
+                       }
+               }
+               $parts = explode('?', $query, $unnamedParameterCount + 1);
+               $query = implode($this->parameterWrapToken . '?' . $this->parameterWrapToken, $parts);
+
+               return $query;
+       }
+
+       /**
+        * Generate a random token that is used to wrap the query markers
+        *
+        * @return string
+        */
+       protected function generateParameterWrapToken() {
+               return '__' . t3lib_div::getRandomHexString(16) . '__';
+       }
 }
 
 
index 845f401..73f5a7b 100644 (file)
@@ -139,7 +139,13 @@ class t3lib_db_PreparedStatementTest extends tx_phpunit_testcase {
                        'php bool FALSE parameter is replaced with 0' => array('SELECT * FROM pages WHERE deleted=?', array(FALSE), 'SELECT * FROM pages WHERE deleted=0'),
                        'php null parameter is replaced with NULL' => array('SELECT * FROM pages WHERE deleted=?', array(NULL), 'SELECT * FROM pages WHERE deleted=NULL'),
                        'string parameter is wrapped in quotes' => array('SELECT * FROM pages WHERE title=?', array('Foo bar'), "SELECT * FROM pages WHERE title='Foo bar'"),
+                       'number as string parameter is wrapped in quotes' => array('SELECT * FROM pages WHERE title=?', array('12'), "SELECT * FROM pages WHERE title='12'"),
                        'string single quotes in parameter are properly escaped' => array('SELECT * FROM pages WHERE title=?', array("'Foo'"), "SELECT * FROM pages WHERE title='\\'Foo\\''"),
+                       'question mark as values with unnamed parameters are properly escaped' => array('SELECT * FROM foo WHERE title=? AND name=?', array("?", "fancy title"), "SELECT * FROM foo WHERE title='?' AND name='fancy title'"),
+                       'parameter name as value is properly escaped' => array('SELECT * FROM foo WHERE title=:name AND name=:title', array(':name'=>":title", ':title'=>"fancy title"), "SELECT * FROM foo WHERE title=':title' AND name='fancy title'"),
+                       'question mark as value of a parameter with a name is properly escaped' => array('SELECT * FROM foo WHERE title=:name AND name=?', array(':name'=>"?", "cool name"), "SELECT * FROM foo WHERE title='?' AND name='cool name'"),
+                       'regular expression back references as values are left untouched' => array('SELECT * FROM foo WHERE title=:name AND name=?', array(':name'=>'\1', '${1}'), "SELECT * FROM foo WHERE title='\\\\1' AND name='\${1}'"),
+                       'unsubstituted question marks do not contain the token wrap' => array('SELECT * FROM foo WHERE title=:name AND question LIKE "%what?" AND name=:title', array(':name'=>'Title', ':title' => 'Name'), "SELECT * FROM foo WHERE title='Title' AND question LIKE \"%what?\" AND name='Name'"),
                );
        }