Fixes for issues found by Semmle

source/pdo_sqlsrv/pdo_dbh.cpp

@@ -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();

source/shared/core_conn.cpp

@@ -704,22 +704,50 @@ bool core_is_conn_opt_value_escaped( _Inout_ const char* value, _Inout_ size_t v
 {
     // 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] == '}' ) {
+    if (value_len > 0 && value[0] == '{' && value[value_len - 1] == '}') {
         ++value;
         value_len -= 2;
     }
-    // check to make sure that all right braces are escaped
-    size_t i = 0;
-    while( ( value[i] != '}' || ( value[i] == '}' && value[i+1] == '}' )) && i < value_len ) {
-        // skip both braces
-        if( value[i] == '}' )
-            ++i;
-        ++i;
+
+    // Check to make sure that all right braces are escaped
+    // First check the case where there is one character
+    if (value_len == 1) {
+        if (value[0] == '}') {
+            return false;
+        }
+        else {
+            return true;
+        }
     }
-    if( i < value_len && value[i] == '}' ) {
+
+    // Check the case where there is one } at the end
+    if (value[value_len - 2] != '}' && value[value_len - 1] == '}') {
         return false;
     }
 
+    // Now run through the string starting at the second position
+    size_t i = 1;
+    while (i < value_len) {
+        // If there are two }, the } is escaped
+        // Otherwise check for one }
+        // Otherwise simply advance the index
+        if (value[i] == '}'&&value[i - 1] == '}') {
+            i += 2;
+
+            // Since i advanced by 2, check the case where three } lie at the end of the string
+            // Note if there are 2 } at the end, i is value_len+1 so does not trigger this
+            if (i == value_len && value[value_len - 1] == '}') {
+                return false;
+            }
+        }
+        else if (value[i - 1] == '}' && value[i] != '}') {
+            return false;
+        }
+        else {
+            ++i;
+        }
+    }
+
     return true;
 }
 

source/shared/core_results.cpp

@@ -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 )

source/shared/core_sqlsrv.h

@@ -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 {
 

source/sqlsrv/init.cpp

@@ -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 )

source/sqlsrv/stmt.cpp

@@ -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" );
 

test/functional/sqlsrv/sqlsrv_escape_braces.phpt

@@ -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.