From 2ac9c30f73ec2e6235c602bed745749a551b4fe2 Mon Sep 17 00:00:00 2001 From: Enrico Zimuel Date: Wed, 2 Sep 2015 12:21:47 +0200 Subject: [PATCH] [ZF2015-08] Fix null byte injection for PDO MsSql This addresses the same issue as found in ZF2014-06, but within the PDO MsSql adapter. Additionally, it fixes transaction tests for that adapter. --- library/Zend/Db/Adapter/Pdo/Abstract.php | 3 +- library/Zend/Db/Adapter/Pdo/Mssql.php | 2 +- tests/TestConfiguration.php.dist | 5 ++- tests/Zend/Db/Adapter/Pdo/MssqlTest.php | 47 +++++------------------- tests/Zend/Db/Adapter/Pdo/TestCommon.php | 10 +++++ tests/Zend/Db/Adapter/TestCommon.php | 5 +-- tests/Zend/Db/TestUtil/Pdo/Mssql.php | 4 +- 7 files changed, 31 insertions(+), 45 deletions(-) diff --git a/library/Zend/Db/Adapter/Pdo/Abstract.php b/library/Zend/Db/Adapter/Pdo/Abstract.php index c764e0a22d..cf2ecbdc2a 100644 --- a/library/Zend/Db/Adapter/Pdo/Abstract.php +++ b/library/Zend/Db/Adapter/Pdo/Abstract.php @@ -292,6 +292,8 @@ protected function _quote($value) if (is_int($value) || is_float($value)) { return $value; } + // Fix for null-byte injection + $value = addcslashes($value, "\000\032"); $this->_connect(); return $this->_connection->quote($value); } @@ -398,4 +400,3 @@ public function getServerVersion() } } } - diff --git a/library/Zend/Db/Adapter/Pdo/Mssql.php b/library/Zend/Db/Adapter/Pdo/Mssql.php index 05092f34ee..04f4aeb3c5 100644 --- a/library/Zend/Db/Adapter/Pdo/Mssql.php +++ b/library/Zend/Db/Adapter/Pdo/Mssql.php @@ -410,7 +410,7 @@ public function lastInsertId($tableName = null, $primaryKey = null) public function getServerVersion() { try { - $stmt = $this->query("SELECT SERVERPROPERTY('productversion')"); + $stmt = $this->query("SELECT CAST(SERVERPROPERTY('productversion') AS VARCHAR)"); $result = $stmt->fetchAll(Zend_Db::FETCH_NUM); if (count($result)) { return $result[0][0]; diff --git a/tests/TestConfiguration.php.dist b/tests/TestConfiguration.php.dist index acc56dc7d0..3c5451326f 100644 --- a/tests/TestConfiguration.php.dist +++ b/tests/TestConfiguration.php.dist @@ -84,7 +84,7 @@ defined('TESTS_ZEND_CACHE_LIBMEMCACHED_WEIGHT') || define('TESTS_ZEND_CACHE_LIBM /** * Zend_Cloud online tests * - * You may need to provide connection details for specific adapters under their + * You may need to provide connection details for specific adapters under their * specific configuration settings elsewhere in this file. */ defined('TESTS_ZEND_CLOUD_STORAGE_WINDOWSAZURE_CONTAINER') || define('TESTS_ZEND_CLOUD_STORAGE_WINDOWSAZURE_CONTAINER', 'simplecloudcontainer'); @@ -133,6 +133,7 @@ defined('TESTS_ZEND_DB_ADAPTER_PDO_MSSQL_HOSTNAME') || define('TESTS_ZEND_DB_ADA defined('TESTS_ZEND_DB_ADAPTER_PDO_MSSQL_USERNAME') || define('TESTS_ZEND_DB_ADAPTER_PDO_MSSQL_USERNAME', null); defined('TESTS_ZEND_DB_ADAPTER_PDO_MSSQL_PASSWORD') || define('TESTS_ZEND_DB_ADAPTER_PDO_MSSQL_PASSWORD', null); defined('TESTS_ZEND_DB_ADAPTER_PDO_MSSQL_DATABASE') || define('TESTS_ZEND_DB_ADAPTER_PDO_MSSQL_DATABASE', 'test'); +defined('TESTS_ZEND_DB_ADAPTER_PDO_MSSQL_PDOTYPE') || define('TESTS_ZEND_DB_ADAPTER_PDO_MSSQL_PDOTYPE', 'dblib'); /** * Zend_Db_Adapter_Pdo_Pgsql @@ -331,7 +332,7 @@ defined('TESTS_ZEND_HTTP_CLIENT_HTTP_PROXY_PASS') || define('TESTS_ZEND_HTTP_CLI /** * Zend_Http_UserAgent tests * - * Location of WURFL library and config file, for testing mobile device + * Location of WURFL library and config file, for testing mobile device * detection. */ defined('TESTS_ZEND_HTTP_USERAGENT_WURFL_LIB_DIR') || define('TESTS_ZEND_HTTP_USERAGENT_WURFL_LIB_DIR', false); diff --git a/tests/Zend/Db/Adapter/Pdo/MssqlTest.php b/tests/Zend/Db/Adapter/Pdo/MssqlTest.php index 9a2d3bd011..bb16a000e9 100644 --- a/tests/Zend/Db/Adapter/Pdo/MssqlTest.php +++ b/tests/Zend/Db/Adapter/Pdo/MssqlTest.php @@ -211,11 +211,13 @@ public function testAdapterTransactionCommit() $bugs = $this->_db->quoteIdentifier('zfbugs'); $bug_id = $this->_db->quoteIdentifier('bug_id'); - // notice the number of rows in connection 2 + $dbConnection1 = $this->_db; + + // notice the number of rows at beginning $count = $this->_db->fetchOne("SELECT COUNT(*) FROM $bugs"); $this->assertEquals(4, $count, 'Expecting to see 4 rows in bugs table (step 1)'); - // start an explicit transaction in connection 1 + // start an explicit transaction $this->_db->beginTransaction(); // delete a row in connection 1 @@ -225,29 +227,12 @@ public function testAdapterTransactionCommit() ); $this->assertEquals(1, $rowsAffected); - // we should still see all rows in connection 2 - // because the DELETE has not been committed yet - $count = $this->_db->fetchOne("SELECT COUNT(*) FROM $bugs"); - $this->assertEquals(4, $count, 'Expecting to still see 4 rows in bugs table (step 2); perhaps Adapter is still in autocommit mode?'); - // commit the DELETE $this->_db->commit(); - // now we should see one fewer rows in connection 2 + // now we should see one fewer rows $count = $this->_db->fetchOne("SELECT COUNT(*) FROM $bugs"); $this->assertEquals(3, $count, 'Expecting to see 3 rows in bugs table after DELETE (step 3)'); - - // delete another row in connection 1 - $rowsAffected = $this->_db->delete( - 'zfbugs', - "$bug_id = 2" - ); - $this->assertEquals(1, $rowsAffected); - - // we should see results immediately, because - // the db connection returns to auto-commit mode - $count = $this->_db->fetchOne("SELECT COUNT(*) FROM $bugs"); - $this->assertEquals(2, $count); } public function testAdapterTransactionRollback() @@ -255,7 +240,7 @@ public function testAdapterTransactionRollback() $bugs = $this->_db->quoteIdentifier('zfbugs'); $bug_id = $this->_db->quoteIdentifier('bug_id'); - // notice the number of rows in connection 2 + // notice the number of rows at beginning $count = $this->_db->fetchOne("SELECT COUNT(*) FROM $bugs"); $this->assertEquals(4, $count, 'Expecting to see 4 rows in bugs table (step 1)'); @@ -269,30 +254,18 @@ public function testAdapterTransactionRollback() ); $this->assertEquals(1, $rowsAffected); - // we should still see all rows in connection 2 + // we should still see 3 rows // because the DELETE has not been committed yet $count = $this->_db->fetchOne("SELECT COUNT(*) FROM $bugs"); - $this->assertEquals(4, $count, 'Expecting to still see 4 rows in bugs table (step 2); perhaps Adapter is still in autocommit mode?'); + $this->assertEquals(3, $count, 'Expecting to see 3 rows in bugs table (step 2)'); // rollback the DELETE $this->_db->rollback(); - // now we should see the same number of rows + // now we should see the original number of rows // because the DELETE was rolled back $count = $this->_db->fetchOne("SELECT COUNT(*) FROM $bugs"); - $this->assertEquals(4, $count, 'Expecting to still see 4 rows in bugs table after DELETE is rolled back (step 3)'); - - // delete another row in connection 1 - $rowsAffected = $this->_db->delete( - 'zfbugs', - "$bug_id = 2" - ); - $this->assertEquals(1, $rowsAffected); - - // we should see results immediately, because - // the db connection returns to auto-commit mode - $count = $this->_db->fetchOne("SELECT COUNT(*) FROM $bugs"); - $this->assertEquals(3, $count, 'Expecting to see 3 rows in bugs table after DELETE (step 4)'); + $this->assertEquals(4, $count, 'Expecting to see 4 rows in bugs table after DELETE is rolled back (step 3)'); } /** diff --git a/tests/Zend/Db/Adapter/Pdo/TestCommon.php b/tests/Zend/Db/Adapter/Pdo/TestCommon.php index 4f96ab4f8e..7c323b0a32 100644 --- a/tests/Zend/Db/Adapter/Pdo/TestCommon.php +++ b/tests/Zend/Db/Adapter/Pdo/TestCommon.php @@ -90,4 +90,14 @@ public function testAdapterExecModifiedNone() $this->assertEquals(0, $affected, "Expected exec() to return zero affected rows; got $affected"); } + + /** + * Test for null byte injection + */ + public function testAdapterQuoteNullByteCharacter() + { + $string = "1\0"; + $value = $this->_db->quote($string); + $this->assertEquals("'1\\000'", $value); + } } diff --git a/tests/Zend/Db/Adapter/TestCommon.php b/tests/Zend/Db/Adapter/TestCommon.php index 3ed5d502c5..e757b26f1c 100644 --- a/tests/Zend/Db/Adapter/TestCommon.php +++ b/tests/Zend/Db/Adapter/TestCommon.php @@ -1571,7 +1571,7 @@ public function testAdapterTransactionCommit() // create a second connection to the same database $dbConnection2 = Zend_Db::factory($this->getDriver(), $this->_util->getParams()); $dbConnection2->getConnection(); - + // notice the number of rows in connection 2 $count = $dbConnection2->fetchOne("SELECT COUNT(*) FROM $bugs"); $this->assertEquals(4, $count, 'Expecting to see 4 rows in bugs table (step 1)'); @@ -2030,11 +2030,10 @@ public function testCharacterSetUtf8() // create test table using no identifier quoting $util->createTable('charsetutf8', array( - 'id' => 'IDENTITY', + 'id' => 'INTEGER NOT NULL', 'stuff' => 'VARCHAR(32)' )); $tableName = $this->_util->getTableName('charsetutf8'); - // insert into the table $numRows = $db->insert($tableName, array( 'id' => 1, diff --git a/tests/Zend/Db/TestUtil/Pdo/Mssql.php b/tests/Zend/Db/TestUtil/Pdo/Mssql.php index 9c1a54cfb3..c20a8c8a98 100644 --- a/tests/Zend/Db/TestUtil/Pdo/Mssql.php +++ b/tests/Zend/Db/TestUtil/Pdo/Mssql.php @@ -45,7 +45,9 @@ public function getParams(array $constants = array()) 'username' => 'TESTS_ZEND_DB_ADAPTER_PDO_MSSQL_USERNAME', 'password' => 'TESTS_ZEND_DB_ADAPTER_PDO_MSSQL_PASSWORD', 'dbname' => 'TESTS_ZEND_DB_ADAPTER_PDO_MSSQL_DATABASE', - 'port' => 'TESTS_ZEND_DB_ADAPTER_PDO_MSSQL_PORT' + 'port' => 'TESTS_ZEND_DB_ADAPTER_PDO_MSSQL_PORT', + 'pdoType' => 'TESTS_ZEND_DB_ADAPTER_PDO_MSSQL_PDOTYPE', + 'charset' => 'TESTS_ZEND_DB_ADAPTER_PDO_MSSQL_CHARSET', ); return parent::getParams($constants);