Skip to content

Parameterize PDO::lastInsertId() sequence-name lookup and fix non-ASCII sequence names #1673

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
David-Engel wants to merge 3 commits into
base: dev
Choose a base branch
from
+38 −10
Draft

Parameterize PDO::lastInsertId() sequence-name lookup and fix non-ASCII sequence names #1673

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ef9e7c5
Fix sequence name binding in PDO::lastInsertId
David-Engel Jun 15, 2026
1c0e1b4
test(pdo): cover non-ASCII sequence names and injection-payload regre…
David-Engel Jun 26, 2026
c0a15b8
test(pdo): set UTF-8 connection encoding for Unicode sequence-name case
David-Engel Jul 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 17 additions & 9 deletions source/pdo_sqlsrv/pdo_dbh.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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<SQLWCHAR> wsql_string;
Expand All @@ -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();
Expand All @@ -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<pdo_sqlsrv_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 );
Expand All @@ -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_cast<decltype(dbh->error_mode)>(prev_err_mode);
Expand All @@ -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<char*>(sqlsrv_malloc(0, sizeof(char), 1)); // return an empty string with a null terminator
Expand Down
22 changes: 21 additions & 1 deletion test/functional/pdo_sqlsrv/pdo_278_lastinsertid_seq.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,14 +55,34 @@ 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

@David-Engel David-Engel Jul 16, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, thanks. Note this test uses connect() (which doesn't force an encoding) rather than PDOConnect(), so it runs under the connection default (UTF-8). Still, I've made it explicit: it now sets PDO::SQLSRV_ATTR_ENCODING to PDO::SQLSRV_ENCODING_UTF8 before the Unicode DDL/DML and the lastInsertId($unicodeSeq) lookup, so the case reliably exercises the fix regardless of the platform system code page. Done in 178d59b.

// 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";
}
dropTable($conn, $tableName1);
dropTable($conn, $tableName2);
$conn->exec("DROP SEQUENCE $sequenceName");
$conn->exec("DROP SEQUENCE [$unicodeSeq]");
unset($stmt);
}
unset($conn);
Expand Down
Loading