Skip to content

Commit 3341925

Browse files
authored
Parameterize PDO::lastInsertId() sequence-name lookup and fix non-ASCII sequence names (#1673)
* Fix sequence name binding in PDO::lastInsertId Previous code didn't correctly handle non-ASCII bytes * test(pdo): cover non-ASCII sequence names and injection-payload regression for lastInsertId Add coverage to pdo_278_lastinsertid_seq.phpt for the parameterized sequence-name lookup: a sequence whose name contains non-ASCII (Unicode) characters now resolves correctly via lastInsertId(), and an injection-style name is treated as a literal value that matches no sequence (returns an empty string) rather than altering the query. * test(pdo): set UTF-8 connection encoding for Unicode sequence-name case The Unicode sequence-name literal is UTF-8. Explicitly set PDO::SQLSRV_ATTR_ENCODING to UTF-8 before the Unicode DDL/DML and the lastInsertId() lookup so the case reliably exercises the fix regardless of the platform's system code page, addressing PR review feedback.
1 parent 5fe2823 commit 3341925

2 files changed

Lines changed: 38 additions & 10 deletions

File tree

source/pdo_sqlsrv/pdo_dbh.cpp

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,7 @@ namespace {
3434

3535
const char LAST_INSERT_ID_QUERY[] = "SELECT @@IDENTITY;";
3636
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)
37-
const char SEQUENCE_CURRENT_VALUE_QUERY[] = "SELECT current_value FROM sys.sequences WHERE name=N'%s'";
38-
const int LAST_INSERT_ID_QUERY_MAX_LEN = sizeof( SEQUENCE_CURRENT_VALUE_QUERY ) + SQL_MAX_SQLSERVERNAME + 2; // include the quotes
37+
const char SEQUENCE_CURRENT_VALUE_QUERY[] = "SELECT current_value FROM sys.sequences WHERE name=?";
3938

4039
// List of PDO supported connection options.
4140
namespace PDOConnOptionNames {
@@ -1630,6 +1629,8 @@ zend_string * pdo_sqlsrv_dbh_last_id(_Inout_ pdo_dbh_t *dbh, _In_ const zend_str
16301629
char idSTR[LAST_INSERT_ID_BUFF_LEN] = { '\0' };
16311630
char* str = NULL;
16321631
SQLLEN cbID = 0;
1632+
zval name_z;
1633+
ZVAL_UNDEF(&name_z);
16331634

16341635
try {
16351636
sqlsrv_malloc_auto_ptr<SQLWCHAR> wsql_string;
@@ -1638,13 +1639,7 @@ zend_string * pdo_sqlsrv_dbh_last_id(_Inout_ pdo_dbh_t *dbh, _In_ const zend_str
16381639
if (name == NULL) {
16391640
wsql_string = utf16_string_from_mbcs_string(SQLSRV_ENCODING_CHAR, LAST_INSERT_ID_QUERY, sizeof(LAST_INSERT_ID_QUERY), &wsql_len);
16401641
} else {
1641-
char buffer[LAST_INSERT_ID_QUERY_MAX_LEN] = { '\0' };
1642-
#if PHP_VERSION_ID < 80100
1643-
snprintf(buffer, LAST_INSERT_ID_QUERY_MAX_LEN, SEQUENCE_CURRENT_VALUE_QUERY, name);
1644-
#else
1645-
snprintf(buffer, LAST_INSERT_ID_QUERY_MAX_LEN, SEQUENCE_CURRENT_VALUE_QUERY, ZSTR_VAL(name));
1646-
#endif
1647-
wsql_string = utf16_string_from_mbcs_string(SQLSRV_ENCODING_CHAR, buffer, sizeof(buffer), &wsql_len);
1642+
wsql_string = utf16_string_from_mbcs_string(SQLSRV_ENCODING_CHAR, SEQUENCE_CURRENT_VALUE_QUERY, sizeof(SEQUENCE_CURRENT_VALUE_QUERY), &wsql_len);
16481643
}
16491644
CHECK_CUSTOM_ERROR(wsql_string == 0, driver_stmt, SQLSRV_ERROR_QUERY_STRING_ENCODING_TRANSLATE, get_last_error_message(), NULL) {
16501645
throw core::CoreException();
@@ -1658,6 +1653,17 @@ zend_string * pdo_sqlsrv_dbh_last_id(_Inout_ pdo_dbh_t *dbh, _In_ const zend_str
16581653
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 );
16591654
driver_stmt->set_func( __FUNCTION__ );
16601655

1656+
// Bind the sequence name using a character parameter with the application-defined encoding
1657+
if (name != NULL) {
1658+
#if PHP_VERSION_ID < 80100
1659+
ZVAL_STRING(&name_z, name);
1660+
#else
1661+
ZVAL_STRINGL(&name_z, ZSTR_VAL(name), ZSTR_LEN(name));
1662+
#endif
1663+
core_sqlsrv_bind_param( driver_stmt, 0 /*param_num*/, SQL_PARAM_INPUT, &name_z, SQLSRV_PHPTYPE_INVALID,
1664+
driver_dbh->encoding(), SQL_UNKNOWN_TYPE, SQLSRV_UNKNOWN_SIZE, 0 );
1665+
}
1666+
16611667
// execute the last insert id query
16621668
core::SQLExecDirectW( driver_stmt, wsql_string );
16631669
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
16691675
}
16701676

16711677
driver_stmt->~sqlsrv_stmt();
1678+
zval_ptr_dtor(&name_z);
16721679
} catch( core::CoreException& ) {
16731680
// restore error handling to its previous mode
16741681
dbh->error_mode = static_cast<decltype(dbh->error_mode)>(prev_err_mode);
@@ -1682,6 +1689,7 @@ zend_string * pdo_sqlsrv_dbh_last_id(_Inout_ pdo_dbh_t *dbh, _In_ const zend_str
16821689
if( driver_stmt ) {
16831690
driver_stmt->~sqlsrv_stmt();
16841691
}
1692+
zval_ptr_dtor(&name_z);
16851693
#if PHP_VERSION_ID < 80100
16861694
*len = 0;
16871695
str = reinterpret_cast<char*>(sqlsrv_malloc(0, sizeof(char), 1)); // return an empty string with a null terminator

test/functional/pdo_sqlsrv/pdo_278_lastinsertid_seq.phpt

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,14 +55,34 @@ try {
5555
// defaults to $tableName2 -- because it returns the last inserted row id value
5656
$lastRow = $conn->lastInsertId();
5757

58-
if ($lastSeq == 3 && $lastRow == 1) {
58+
// The sequence name passed to lastInsertId() is bound as a parameter. Verify
59+
// a sequence whose name contains non-ASCII (Unicode) characters resolves
60+
// correctly -- previously the name was interpreted using the system code page
61+
// and such lookups could fail to match.
62+
// The name literal below is UTF-8, so ensure the connection encoding is UTF-8
63+
// for the Unicode DDL/DML and the lastInsertId() lookup regardless of the
64+
// platform's system code page.
65+
$conn->setAttribute(PDO::SQLSRV_ATTR_ENCODING, PDO::SQLSRV_ENCODING_UTF8);
66+
$unicodeSeq = 'séquence_Ñ_日本';
67+
$conn->exec("IF OBJECT_ID(N'$unicodeSeq', 'SO') IS NOT NULL DROP SEQUENCE [$unicodeSeq]");
68+
$conn->exec("CREATE SEQUENCE [$unicodeSeq] AS INTEGER START WITH 1 INCREMENT BY 1 MINVALUE 1 MAXVALUE 100");
69+
$conn->query("SELECT NEXT VALUE FOR [$unicodeSeq]")->fetchColumn();
70+
$lastUnicodeSeq = $conn->lastInsertId($unicodeSeq);
71+
72+
// Because the name is parameterized, a SQL-injection payload is treated as a
73+
// literal sequence name: it matches no sequence and returns an empty string
74+
// instead of altering the query.
75+
$lastInjection = $conn->lastInsertId("x' UNION ALL SELECT DB_NAME()--");
76+
77+
if ($lastSeq == 3 && $lastRow == 1 && $lastUnicodeSeq == 1 && $lastInjection === '') {
5978
echo "Done\n";
6079
} else {
6180
echo "sequence value or identity does not match as expected\n";
6281
}
6382
dropTable($conn, $tableName1);
6483
dropTable($conn, $tableName2);
6584
$conn->exec("DROP SEQUENCE $sequenceName");
85+
$conn->exec("DROP SEQUENCE [$unicodeSeq]");
6686
unset($stmt);
6787
}
6888
unset($conn);

0 commit comments

Comments
 (0)