Skip to content
This repository was archived by the owner on Oct 2, 2019. It is now read-only.

Commit 2ac9c30

Browse files
ezimuelweierophinney
ezimuel
authored and
weierophinney
committed
[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.
1 parent f61d12c commit 2ac9c30

File tree

7 files changed

+31
-45
lines changed

7 files changed

+31
-45
lines changed

‎library/Zend/Db/Adapter/Pdo/Abstract.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,8 @@ protected function _quote($value)
292292
if (is_int($value) || is_float($value)) {
293293
return $value;
294294
}
295+
// Fix for null-byte injection
296+
$value = addcslashes($value, "\000\032");
295297
$this->_connect();
296298
return $this->_connection->quote($value);
297299
}
@@ -398,4 +400,3 @@ public function getServerVersion()
398400
}
399401
}
400402
}
401-

‎library/Zend/Db/Adapter/Pdo/Mssql.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -410,7 +410,7 @@ public function lastInsertId($tableName = null, $primaryKey = null)
410410
public function getServerVersion()
411411
{
412412
try {
413-
$stmt = $this->query("SELECT SERVERPROPERTY('productversion')");
413+
$stmt = $this->query("SELECT CAST(SERVERPROPERTY('productversion') AS VARCHAR)");
414414
$result = $stmt->fetchAll(Zend_Db::FETCH_NUM);
415415
if (count($result)) {
416416
return $result[0][0];

‎tests/TestConfiguration.php.dist

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ defined('TESTS_ZEND_CACHE_LIBMEMCACHED_WEIGHT') || define('TESTS_ZEND_CACHE_LIBM
8484
/**
8585
* Zend_Cloud online tests
8686
*
87-
* You may need to provide connection details for specific adapters under their
87+
* You may need to provide connection details for specific adapters under their
8888
* specific configuration settings elsewhere in this file.
8989
*/
9090
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
133133
defined('TESTS_ZEND_DB_ADAPTER_PDO_MSSQL_USERNAME') || define('TESTS_ZEND_DB_ADAPTER_PDO_MSSQL_USERNAME', null);
134134
defined('TESTS_ZEND_DB_ADAPTER_PDO_MSSQL_PASSWORD') || define('TESTS_ZEND_DB_ADAPTER_PDO_MSSQL_PASSWORD', null);
135135
defined('TESTS_ZEND_DB_ADAPTER_PDO_MSSQL_DATABASE') || define('TESTS_ZEND_DB_ADAPTER_PDO_MSSQL_DATABASE', 'test');
136+
defined('TESTS_ZEND_DB_ADAPTER_PDO_MSSQL_PDOTYPE') || define('TESTS_ZEND_DB_ADAPTER_PDO_MSSQL_PDOTYPE', 'dblib');
136137

137138
/**
138139
* Zend_Db_Adapter_Pdo_Pgsql
@@ -331,7 +332,7 @@ defined('TESTS_ZEND_HTTP_CLIENT_HTTP_PROXY_PASS') || define('TESTS_ZEND_HTTP_CLI
331332
/**
332333
* Zend_Http_UserAgent tests
333334
*
334-
* Location of WURFL library and config file, for testing mobile device
335+
* Location of WURFL library and config file, for testing mobile device
335336
* detection.
336337
*/
337338
defined('TESTS_ZEND_HTTP_USERAGENT_WURFL_LIB_DIR') || define('TESTS_ZEND_HTTP_USERAGENT_WURFL_LIB_DIR', false);

‎tests/Zend/Db/Adapter/Pdo/MssqlTest.php

Lines changed: 10 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -211,11 +211,13 @@ public function testAdapterTransactionCommit()
211211
$bugs = $this->_db->quoteIdentifier('zfbugs');
212212
$bug_id = $this->_db->quoteIdentifier('bug_id');
213213

214-
// notice the number of rows in connection 2
214+
$dbConnection1 = $this->_db;
215+
216+
// notice the number of rows at beginning
215217
$count = $this->_db->fetchOne("SELECT COUNT(*) FROM $bugs");
216218
$this->assertEquals(4, $count, 'Expecting to see 4 rows in bugs table (step 1)');
217219

218-
// start an explicit transaction in connection 1
220+
// start an explicit transaction
219221
$this->_db->beginTransaction();
220222

221223
// delete a row in connection 1
@@ -225,37 +227,20 @@ public function testAdapterTransactionCommit()
225227
);
226228
$this->assertEquals(1, $rowsAffected);
227229

228-
// we should still see all rows in connection 2
229-
// because the DELETE has not been committed yet
230-
$count = $this->_db->fetchOne("SELECT COUNT(*) FROM $bugs");
231-
$this->assertEquals(4, $count, 'Expecting to still see 4 rows in bugs table (step 2); perhaps Adapter is still in autocommit mode?');
232-
233230
// commit the DELETE
234231
$this->_db->commit();
235232

236-
// now we should see one fewer rows in connection 2
233+
// now we should see one fewer rows
237234
$count = $this->_db->fetchOne("SELECT COUNT(*) FROM $bugs");
238235
$this->assertEquals(3, $count, 'Expecting to see 3 rows in bugs table after DELETE (step 3)');
239-
240-
// delete another row in connection 1
241-
$rowsAffected = $this->_db->delete(
242-
'zfbugs',
243-
"$bug_id = 2"
244-
);
245-
$this->assertEquals(1, $rowsAffected);
246-
247-
// we should see results immediately, because
248-
// the db connection returns to auto-commit mode
249-
$count = $this->_db->fetchOne("SELECT COUNT(*) FROM $bugs");
250-
$this->assertEquals(2, $count);
251236
}
252237

253238
public function testAdapterTransactionRollback()
254239
{
255240
$bugs = $this->_db->quoteIdentifier('zfbugs');
256241
$bug_id = $this->_db->quoteIdentifier('bug_id');
257242

258-
// notice the number of rows in connection 2
243+
// notice the number of rows at beginning
259244
$count = $this->_db->fetchOne("SELECT COUNT(*) FROM $bugs");
260245
$this->assertEquals(4, $count, 'Expecting to see 4 rows in bugs table (step 1)');
261246

@@ -269,30 +254,18 @@ public function testAdapterTransactionRollback()
269254
);
270255
$this->assertEquals(1, $rowsAffected);
271256

272-
// we should still see all rows in connection 2
257+
// we should still see 3 rows
273258
// because the DELETE has not been committed yet
274259
$count = $this->_db->fetchOne("SELECT COUNT(*) FROM $bugs");
275-
$this->assertEquals(4, $count, 'Expecting to still see 4 rows in bugs table (step 2); perhaps Adapter is still in autocommit mode?');
260+
$this->assertEquals(3, $count, 'Expecting to see 3 rows in bugs table (step 2)');
276261

277262
// rollback the DELETE
278263
$this->_db->rollback();
279264

280-
// now we should see the same number of rows
265+
// now we should see the original number of rows
281266
// because the DELETE was rolled back
282267
$count = $this->_db->fetchOne("SELECT COUNT(*) FROM $bugs");
283-
$this->assertEquals(4, $count, 'Expecting to still see 4 rows in bugs table after DELETE is rolled back (step 3)');
284-
285-
// delete another row in connection 1
286-
$rowsAffected = $this->_db->delete(
287-
'zfbugs',
288-
"$bug_id = 2"
289-
);
290-
$this->assertEquals(1, $rowsAffected);
291-
292-
// we should see results immediately, because
293-
// the db connection returns to auto-commit mode
294-
$count = $this->_db->fetchOne("SELECT COUNT(*) FROM $bugs");
295-
$this->assertEquals(3, $count, 'Expecting to see 3 rows in bugs table after DELETE (step 4)');
268+
$this->assertEquals(4, $count, 'Expecting to see 4 rows in bugs table after DELETE is rolled back (step 3)');
296269
}
297270

298271
/**

‎tests/Zend/Db/Adapter/Pdo/TestCommon.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,4 +90,14 @@ public function testAdapterExecModifiedNone()
9090
$this->assertEquals(0, $affected,
9191
"Expected exec() to return zero affected rows; got $affected");
9292
}
93+
94+
/**
95+
* Test for null byte injection
96+
*/
97+
public function testAdapterQuoteNullByteCharacter()
98+
{
99+
$string = "1\0";
100+
$value = $this->_db->quote($string);
101+
$this->assertEquals("'1\\000'", $value);
102+
}
93103
}

‎tests/Zend/Db/Adapter/TestCommon.php

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1571,7 +1571,7 @@ public function testAdapterTransactionCommit()
15711571
// create a second connection to the same database
15721572
$dbConnection2 = Zend_Db::factory($this->getDriver(), $this->_util->getParams());
15731573
$dbConnection2->getConnection();
1574-
1574+
15751575
// notice the number of rows in connection 2
15761576
$count = $dbConnection2->fetchOne("SELECT COUNT(*) FROM $bugs");
15771577
$this->assertEquals(4, $count, 'Expecting to see 4 rows in bugs table (step 1)');
@@ -2030,11 +2030,10 @@ public function testCharacterSetUtf8()
20302030

20312031
// create test table using no identifier quoting
20322032
$util->createTable('charsetutf8', array(
2033-
'id' => 'IDENTITY',
2033+
'id' => 'INTEGER NOT NULL',
20342034
'stuff' => 'VARCHAR(32)'
20352035
));
20362036
$tableName = $this->_util->getTableName('charsetutf8');
2037-
20382037
// insert into the table
20392038
$numRows = $db->insert($tableName, array(
20402039
'id' => 1,

‎tests/Zend/Db/TestUtil/Pdo/Mssql.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,9 @@ public function getParams(array $constants = array())
4545
'username' => 'TESTS_ZEND_DB_ADAPTER_PDO_MSSQL_USERNAME',
4646
'password' => 'TESTS_ZEND_DB_ADAPTER_PDO_MSSQL_PASSWORD',
4747
'dbname' => 'TESTS_ZEND_DB_ADAPTER_PDO_MSSQL_DATABASE',
48-
'port' => 'TESTS_ZEND_DB_ADAPTER_PDO_MSSQL_PORT'
48+
'port' => 'TESTS_ZEND_DB_ADAPTER_PDO_MSSQL_PORT',
49+
'pdoType' => 'TESTS_ZEND_DB_ADAPTER_PDO_MSSQL_PDOTYPE',
50+
'charset' => 'TESTS_ZEND_DB_ADAPTER_PDO_MSSQL_CHARSET',
4951
);
5052

5153
return parent::getParams($constants);

0 commit comments

Comments
 (0)