[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.
zendframework · Sep 15, 2015 · 2ac9c30 · 2ac9c30
1 parent f61d12c
commit 2ac9c30
Show file tree

Hide file tree

Showing 7 changed files with 31 additions and 45 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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,37 +227,20 @@ 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()
     {
         $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
@@ -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
@@ -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
@@ -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);