dprevot/php-sqlsrv
1
0
Fork 0
Code Issues Pull requests Packages Projects Releases Wiki Activity

Fixed for issues found by Semmle (#1011)

Browse source 
* Removed unneeded constants

* Fixed sqlsrv_free_stmt argument info

* Fixed brace escape to avoid buffer overflow

* Fixed brace escape and added test

* Debugging test failure on Bamboo

* Removed debugging output

* Debugging test failure on Bamboo

* Removed debugging output

* Added more test cases

* Changed range check to use strchr

* Added pdo test

* Fixed test and formatting
This commit is contained in:
David Puglielli 2019-07-23 15:12:55 -07:00 committed by GitHub
parent b839ede878
commit 1a2b49393c
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 165 additions and 24 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
source/pdo_sqlsrv/pdo_dbh.cpp
View file

@ -1692,7 +1692,6 @@ void validate_stmt_options( _Inout_ sqlsrv_context& ctx, _Inout_ zval* stmt_opti
ZEND_HASH_FOREACH_KEY_VAL( options_ht, int_key, key, data ) {
int type = HASH_KEY_NON_EXISTENT;
int result = 0;
type = key ? HASH_KEY_IS_STRING : HASH_KEY_IS_LONG;
CHECK_CUSTOM_ERROR(( type != HASH_KEY_IS_LONG ), ctx, PDO_SQLSRV_ERROR_INVALID_STMT_OPTION ) {
throw core::CoreException();

36
source/shared/core_conn.cpp
View file

@ -702,22 +702,32 @@ void core_sqlsrv_get_client_info( _Inout_ sqlsrv_conn* conn, _Out_ zval *client_
bool core_is_conn_opt_value_escaped( _Inout_ const char* value, _Inout_ size_t value_len )
{
// if the value is already quoted, then only analyse the part inside the quotes and return it as
// unquoted since we quote it when adding it to the connection string.
if( value_len > 0 && value[0] == '{' && value[value_len - 1] == '}' ) {
++value;
if (value_len == 0) {
return true;
}
if (value_len == 1) {
return (value[0] != '}');
}
const char *pstr = value;
if (value_len > 0 && value[0] == '{' && value[value_len - 1] == '}') {
pstr = ++value;
value_len -= 2;
}
// check to make sure that all right braces are escaped
const char *pch = strchr(pstr, '}');
size_t i = 0;
while( ( value[i] != '}' || ( value[i] == '}' && value[i+1] == '}' )) && i < value_len ) {
// skip both braces
if( value[i] == '}' )
++i;
++i;
}
if( i < value_len && value[i] == '}' ) {
return false;
while (pch != NULL && i < value_len) {
i = pch - pstr + 1;
if (i == value_len || (i < value_len && pstr[i] != '}')) {
return false;
}
i++; // skip the brace
pch = strchr(pch + 2, '}'); // continue searching
}
return true;

2
source/shared/core_results.cpp
View file

@ -243,14 +243,12 @@ std::string getUTF8StringFromString( _In_z_ const SQLWCHAR* source )
{
// convert to regular character string first
char c_str[4] = "";
mbstate_t mbs;
SQLLEN i = 0;
std::string str;
while ( source[i] )
{
memset( c_str, 0, sizeof( c_str ) );
DWORD rc;
int cch = 0;
errno_t err = mplat_wctomb_s( &cch, c_str, sizeof( c_str ), source[i++] );
if ( cch > 0 && err == ERROR_SUCCESS )

3
source/shared/core_sqlsrv.h
View file

@ -170,7 +170,6 @@ OACR_WARNING_POP
// constants for maximums in SQL Server
const int SS_MAXCOLNAMELEN = 128;
const int SQL_SERVER_MAX_FIELD_SIZE = 8000;
const int SQL_SERVER_MAX_PRECISION = 38;
const int SQL_SERVER_MAX_TYPE_SIZE = 0;
const int SQL_SERVER_MAX_PARAMS = 2100;
const int SQL_SERVER_MAX_MONEY_SCALE = 4;
@ -998,8 +997,6 @@ class sqlsrv_context {
SQLSRV_ENCODING encoding_; // encoding of the context
};
const int SQLSRV_OS_VISTA_OR_LATER = 6; // major version for Vista
// maps an IANA encoding to a code page
struct sqlsrv_encoding {

2
source/sqlsrv/init.cpp
View file

@ -219,7 +219,7 @@ zend_function_entry sqlsrv_functions[] = {
PHP_FE( sqlsrv_client_info, sqlsrv_client_info_arginfo )
PHP_FE( sqlsrv_server_info, sqlsrv_server_info_arginfo )
PHP_FE( sqlsrv_cancel, sqlsrv_cancel_arginfo )
PHP_FE( sqlsrv_free_stmt, sqlsrv_close_arginfo )
PHP_FE( sqlsrv_free_stmt, sqlsrv_free_stmt_arginfo )
PHP_FE( sqlsrv_field_metadata, sqlsrv_field_metadata_arginfo )
PHP_FE( sqlsrv_send_stream_data, sqlsrv_send_stream_data_arginfo )
PHP_FE( SQLSRV_SQLTYPE_BINARY, sqlsrv_sqltype_size_arginfo )

4
source/sqlsrv/stmt.cpp
View file

@ -42,7 +42,6 @@ unsigned int current_log_subsystem = LOG_STMT;
// constants used as invalid types for type errors
const zend_uchar PHPTYPE_INVALID = SQLSRV_PHPTYPE_INVALID;
const int SQLTYPE_INVALID = 0;
const int SQLSRV_INVALID_PRECISION = -1;
const SQLUINTEGER SQLSRV_INVALID_SIZE = (~1U);
const int SQLSRV_INVALID_SCALE = -1;
@ -51,8 +50,6 @@ const int SQLSRV_SIZE_MAX_TYPE = -1;
// constants for maximums in SQL Server
const int SQL_SERVER_MAX_FIELD_SIZE = 8000;
const int SQL_SERVER_MAX_PRECISION = 38;
const int SQL_SERVER_DEFAULT_PRECISION = 18;
const int SQL_SERVER_DEFAULT_SCALE = 0;
// default class used when no class is specified by sqlsrv_fetch_object
const char STDCLASS_NAME[] = "stdclass";
@ -470,7 +467,6 @@ PHP_FUNCTION( sqlsrv_fetch_array )
PHP_FUNCTION( sqlsrv_field_metadata )
{
sqlsrv_stmt* stmt = NULL;
SQLSMALLINT num_cols = -1;
LOG_FUNCTION( "sqlsrv_field_metadata" );

71
test/functional/pdo_sqlsrv/pdo_escape_braces.phpt Normal file
View file

@ -0,0 +1,71 @@
--TEST--
Test that right braces are escaped correctly and that error messages are correct when they're not
--SKIPIF--
<?php require('skipif.inc'); ?>
--FILE--
<?php
$server = 'fakeserver';
$uid = 'sa';
$password = 'fakepassword';
// If the braces are fine, then we expect the connection to fail with a login timeout error
$braceError = "An unescaped right brace (}) was found";
$connError = (strtoupper(substr(php_uname('s'), 0, 3)) === 'WIN') ? "Could not open a connection to SQL Server" : "Login timeout expired";
// Every combination of one, two, three, or more right braces I can think of
$testStrings = array(array("}", $braceError),
array("{", $connError),
array("{t}", $connError),
array("{}}", $braceError),
array("}}", $connError),
array("}}}", $braceError),
array("}}}}", $connError),
array("{}}}", $connError),
array("}{", $braceError),
array("}{{", $braceError),
array("test", $connError),
array("{test}", $connError),
array("{test", $connError),
array("test}", $braceError),
array("{{test}}", $braceError),
array("{{test}", $connError),
array("{{test", $connError),
array("test}}", $connError),
array("{test}}", $braceError),
array("test}}}", $braceError),
array("{test}}}", $connError),
array("{test}}}}", $braceError),
array("{test}}}}}", $connError),
array("{test}}}}}}", $braceError),
array("te}st", $braceError),
array("{te}st}", $braceError),
array("{te}}st}", $connError),
array("{te}}}st}", $braceError),
array("te}}s}t", $braceError),
array("te}}s}}t", $connError),
array("te}}}st", $braceError),
array("te}}}}st", $connError),
array("tes}}t", $connError),
array("te}s}}t", $braceError),
array("tes}}t}}", $connError),
array("tes}}t}}}", $braceError),
array("tes}t}}", $braceError),
);
foreach ($testStrings as $test) {
try {
$conn = new PDO("sqlsrv:Server=".$server.";LoginTimeout=1;", $test[0], $password);
} catch (PDOException $e) {
if (strpos($e->getMessage(), $test[1]) === false) {
print_r("Wrong error message returned for test string ".$test[0].". Expected ".$test[1].", actual output:\n");
print_r($e->getMessage);
echo "\n";
}
}
}
echo "Done.\n";
?>
--EXPECT--
Done.

70
test/functional/sqlsrv/sqlsrv_escape_braces.phpt Normal file
View file

@ -0,0 +1,70 @@
--TEST--
Test that right braces are escaped correctly and that error messages are correct when they're not
--SKIPIF--
<?php require('skipif.inc'); ?>
--FILE--
<?php
$server = 'fakeserver';
$uid = 'sa';
$password = 'fakepassword';
// If the braces are fine, then we expect the connection to fail with a login timeout error
$braceError = "An unescaped right brace (}) was found";
$connError = (strtoupper(substr(php_uname('s'), 0, 3)) === 'WIN') ? "Could not open a connection to SQL Server" : "Login timeout expired";
// Every combination of one, two, three, or more right braces I can think of
$testStrings = array(array("}", $braceError),
array("{", $connError),
array("{t}", $connError),
array("{}}", $braceError),
array("}}", $connError),
array("}}}", $braceError),
array("}}}}", $connError),
array("{}}}", $connError),
array("}{", $braceError),
array("}{{", $braceError),
array("test", $connError),
array("{test}", $connError),
array("{test", $connError),
array("test}", $braceError),
array("{{test}}", $braceError),
array("{{test}", $connError),
array("{{test", $connError),
array("test}}", $connError),
array("{test}}", $braceError),
array("test}}}", $braceError),
array("{test}}}", $connError),
array("{test}}}}", $braceError),
array("{test}}}}}", $connError),
array("{test}}}}}}", $braceError),
array("te}st", $braceError),
array("{te}st}", $braceError),
array("{te}}st}", $connError),
array("{te}}}st}", $braceError),
array("te}}s}t", $braceError),
array("te}}s}}t", $connError),
array("te}}}st", $braceError),
array("te}}}}st", $connError),
array("tes}}t", $connError),
array("te}s}}t", $braceError),
array("tes}}t}}", $connError),
array("tes}}t}}}", $braceError),
array("tes}t}}", $braceError),
);
foreach ($testStrings as $test) {
$conn = sqlsrv_connect($server, array('uid'=>$test[0], 'pwd'=>$password, 'LoginTimeout'=>1));
if (strpos(sqlsrv_errors()[0][2], $test[1]) === false) {
print_r("Wrong error message returned for test string ".$test[0].". Expected ".$test[1].", actual output:\n");
print_r(sqlsrv_errors());
}
unset($conn);
}
echo "Done.\n";
?>
--EXPECT--
Done.