diff --git a/source/pdo_sqlsrv/pdo_dbh.cpp b/source/pdo_sqlsrv/pdo_dbh.cpp index 466980acc..d9f7cceb8 100644 --- a/source/pdo_sqlsrv/pdo_dbh.cpp +++ b/source/pdo_sqlsrv/pdo_dbh.cpp @@ -34,8 +34,7 @@ namespace { const char LAST_INSERT_ID_QUERY[] = "SELECT @@IDENTITY;"; const size_t LAST_INSERT_ID_BUFF_LEN = 50; // size of the buffer to hold the string value of the last inserted id, which may be an int, bigint, decimal(p,0) or numeric(p,0) -const char SEQUENCE_CURRENT_VALUE_QUERY[] = "SELECT current_value FROM sys.sequences WHERE name=N'%s'"; -const int LAST_INSERT_ID_QUERY_MAX_LEN = sizeof( SEQUENCE_CURRENT_VALUE_QUERY ) + SQL_MAX_SQLSERVERNAME + 2; // include the quotes +const char SEQUENCE_CURRENT_VALUE_QUERY[] = "SELECT current_value FROM sys.sequences WHERE name=?"; // List of PDO supported connection options. namespace PDOConnOptionNames { @@ -1630,6 +1629,8 @@ zend_string * pdo_sqlsrv_dbh_last_id(_Inout_ pdo_dbh_t *dbh, _In_ const zend_str char idSTR[LAST_INSERT_ID_BUFF_LEN] = { '\0' }; char* str = NULL; SQLLEN cbID = 0; + zval name_z; + ZVAL_UNDEF(&name_z); try { sqlsrv_malloc_auto_ptr wsql_string; @@ -1638,13 +1639,7 @@ zend_string * pdo_sqlsrv_dbh_last_id(_Inout_ pdo_dbh_t *dbh, _In_ const zend_str if (name == NULL) { wsql_string = utf16_string_from_mbcs_string(SQLSRV_ENCODING_CHAR, LAST_INSERT_ID_QUERY, sizeof(LAST_INSERT_ID_QUERY), &wsql_len); } else { - char buffer[LAST_INSERT_ID_QUERY_MAX_LEN] = { '\0' }; -#if PHP_VERSION_ID < 80100 - snprintf(buffer, LAST_INSERT_ID_QUERY_MAX_LEN, SEQUENCE_CURRENT_VALUE_QUERY, name); -#else - snprintf(buffer, LAST_INSERT_ID_QUERY_MAX_LEN, SEQUENCE_CURRENT_VALUE_QUERY, ZSTR_VAL(name)); -#endif - wsql_string = utf16_string_from_mbcs_string(SQLSRV_ENCODING_CHAR, buffer, sizeof(buffer), &wsql_len); + wsql_string = utf16_string_from_mbcs_string(SQLSRV_ENCODING_CHAR, SEQUENCE_CURRENT_VALUE_QUERY, sizeof(SEQUENCE_CURRENT_VALUE_QUERY), &wsql_len); } CHECK_CUSTOM_ERROR(wsql_string == 0, driver_stmt, SQLSRV_ERROR_QUERY_STRING_ENCODING_TRANSLATE, get_last_error_message(), NULL) { throw core::CoreException(); @@ -1658,6 +1653,17 @@ zend_string * pdo_sqlsrv_dbh_last_id(_Inout_ pdo_dbh_t *dbh, _In_ const zend_str driver_stmt = core_sqlsrv_create_stmt( driver_dbh, core::allocate_stmt, NULL /*options_ht*/, NULL /*valid_stmt_opts*/, pdo_sqlsrv_handle_stmt_error, &temp_stmt ); driver_stmt->set_func( __FUNCTION__ ); + // Bind the sequence name using a character parameter with the application-defined encoding + if (name != NULL) { +#if PHP_VERSION_ID < 80100 + ZVAL_STRING(&name_z, name); +#else + ZVAL_STRINGL(&name_z, ZSTR_VAL(name), ZSTR_LEN(name)); +#endif + core_sqlsrv_bind_param( driver_stmt, 0 /*param_num*/, SQL_PARAM_INPUT, &name_z, SQLSRV_PHPTYPE_INVALID, + driver_dbh->encoding(), SQL_UNKNOWN_TYPE, SQLSRV_UNKNOWN_SIZE, 0 ); + } + // execute the last insert id query core::SQLExecDirectW( driver_stmt, wsql_string ); core::SQLFetchScroll( driver_stmt, SQL_FETCH_NEXT, 0 ); @@ -1669,6 +1675,7 @@ zend_string * pdo_sqlsrv_dbh_last_id(_Inout_ pdo_dbh_t *dbh, _In_ const zend_str } driver_stmt->~sqlsrv_stmt(); + zval_ptr_dtor(&name_z); } catch( core::CoreException& ) { // restore error handling to its previous mode dbh->error_mode = static_casterror_mode)>(prev_err_mode); @@ -1682,6 +1689,7 @@ zend_string * pdo_sqlsrv_dbh_last_id(_Inout_ pdo_dbh_t *dbh, _In_ const zend_str if( driver_stmt ) { driver_stmt->~sqlsrv_stmt(); } + zval_ptr_dtor(&name_z); #if PHP_VERSION_ID < 80100 *len = 0; str = reinterpret_cast(sqlsrv_malloc(0, sizeof(char), 1)); // return an empty string with a null terminator diff --git a/test/functional/pdo_sqlsrv/pdo_278_lastinsertid_seq.phpt b/test/functional/pdo_sqlsrv/pdo_278_lastinsertid_seq.phpt index 774463659..0d42685e5 100644 --- a/test/functional/pdo_sqlsrv/pdo_278_lastinsertid_seq.phpt +++ b/test/functional/pdo_sqlsrv/pdo_278_lastinsertid_seq.phpt @@ -55,7 +55,26 @@ try { // defaults to $tableName2 -- because it returns the last inserted row id value $lastRow = $conn->lastInsertId(); - if ($lastSeq == 3 && $lastRow == 1) { + // The sequence name passed to lastInsertId() is bound as a parameter. Verify + // a sequence whose name contains non-ASCII (Unicode) characters resolves + // correctly -- previously the name was interpreted using the system code page + // and such lookups could fail to match. + // The name literal below is UTF-8, so ensure the connection encoding is UTF-8 + // for the Unicode DDL/DML and the lastInsertId() lookup regardless of the + // platform's system code page. + $conn->setAttribute(PDO::SQLSRV_ATTR_ENCODING, PDO::SQLSRV_ENCODING_UTF8); + $unicodeSeq = 'séquence_Ñ_日本'; + $conn->exec("IF OBJECT_ID(N'$unicodeSeq', 'SO') IS NOT NULL DROP SEQUENCE [$unicodeSeq]"); + $conn->exec("CREATE SEQUENCE [$unicodeSeq] AS INTEGER START WITH 1 INCREMENT BY 1 MINVALUE 1 MAXVALUE 100"); + $conn->query("SELECT NEXT VALUE FOR [$unicodeSeq]")->fetchColumn(); + $lastUnicodeSeq = $conn->lastInsertId($unicodeSeq); + + // Because the name is parameterized, a SQL-injection payload is treated as a + // literal sequence name: it matches no sequence and returns an empty string + // instead of altering the query. + $lastInjection = $conn->lastInsertId("x' UNION ALL SELECT DB_NAME()--"); + + if ($lastSeq == 3 && $lastRow == 1 && $lastUnicodeSeq == 1 && $lastInjection === '') { echo "Done\n"; } else { echo "sequence value or identity does not match as expected\n"; @@ -63,6 +82,7 @@ try { dropTable($conn, $tableName1); dropTable($conn, $tableName2); $conn->exec("DROP SEQUENCE $sequenceName"); + $conn->exec("DROP SEQUENCE [$unicodeSeq]"); unset($stmt); } unset($conn);