diff --git a/NEWS b/NEWS index 26abaf2f03ab..8121492e41b9 100644 --- a/NEWS +++ b/NEWS @@ -23,6 +23,10 @@ PHP NEWS or never terminates). (Ilia Alshanetsky) . Fixed memory leak in JIT ASSIGN_DIM when the assignment fails. (Jorg Sowa) +- PDO_Firebird: + . Fixed stack buffer overflow with long connection user names, passwords, + charsets, or roles. (Ilia Alshanetsky) + - Streams: . Fixed bug GH-22063 (Stream filter chain UAF on self-removal during callback). (Ilia Alshanetsky) diff --git a/ext/pdo_firebird/firebird_driver.c b/ext/pdo_firebird/firebird_driver.c index e45a9108f726..7faace6ee5e7 100644 --- a/ext/pdo_firebird/firebird_driver.c +++ b/ext/pdo_firebird/firebird_driver.c @@ -1376,7 +1376,6 @@ static int pdo_firebird_handle_factory(pdo_dbh_t *dbh, zval *driver_options) /* { "password", NULL, 0 } }; int i, ret = 0; - short buf_len = 256, dpb_len; pdo_firebird_db_handle *H = dbh->driver_data = pecalloc(1,sizeof(*H),dbh->is_persistent); @@ -1407,8 +1406,12 @@ static int pdo_firebird_handle_factory(pdo_dbh_t *dbh, zval *driver_options) /* do { static char const dpb_flags[] = { isc_dpb_user_name, isc_dpb_password, isc_dpb_lc_ctype, isc_dpb_sql_role_name }; + static char const *const dpb_names[] = { "user", "password", "charset", "role" }; char const *dpb_values[] = { dbh->username, dbh->password, vars[1].optval, vars[2].optval }; char dpb_buffer[256] = { isc_dpb_version1 }, *dpb; + size_t buf_len = sizeof(dpb_buffer) - 1; + size_t dpb_value_len; + int dpb_len; if (EG(exception)) { break; @@ -1418,14 +1421,26 @@ static int pdo_firebird_handle_factory(pdo_dbh_t *dbh, zval *driver_options) /* /* loop through all the provided arguments and set dpb fields accordingly */ for (i = 0; i < sizeof(dpb_flags); ++i) { - if (dpb_values[i] && buf_len > 0) { - dpb_len = slprintf(dpb, buf_len, "%c%c%s", dpb_flags[i], (unsigned char)strlen(dpb_values[i]), + if (dpb_values[i]) { + dpb_value_len = strlen(dpb_values[i]); + if (dpb_value_len + 2 >= buf_len) { + zend_throw_exception_ex(php_pdo_get_exception(), 0, + "Firebird database parameter \"%s\" is too long", dpb_names[i]); + break; + } + ZEND_ASSERT(dpb_value_len <= UCHAR_MAX); + + dpb_len = slprintf(dpb, buf_len, "%c%c%s", dpb_flags[i], (unsigned char)dpb_value_len, dpb_values[i]); dpb += dpb_len; buf_len -= dpb_len; } } + if (EG(exception)) { + break; + } + H->sql_dialect = PDO_FB_DIALECT; if (vars[3].optval) { H->sql_dialect = atoi(vars[3].optval); diff --git a/ext/pdo_firebird/tests/dpb_overflow.phpt b/ext/pdo_firebird/tests/dpb_overflow.phpt new file mode 100644 index 000000000000..27d1b3e5d15d --- /dev/null +++ b/ext/pdo_firebird/tests/dpb_overflow.phpt @@ -0,0 +1,27 @@ +--TEST-- +PDO_Firebird rejects values that overflow the database parameter buffer +--EXTENSIONS-- +pdo_firebird +--SKIPIF-- + +--FILE-- + [str_repeat('u', 256), PDO_FIREBIRD_TEST_PASS], + 'DPB capacity overflow' => [str_repeat('u', 200), str_repeat('p', 52)], +]; + +foreach ($tests as $description => [$username, $password]) { + try { + new PDO(PDO_FIREBIRD_TEST_DSN, $username, $password); + echo "$description: accepted\n"; + } catch (PDOException $e) { + echo "$description: ", $e::class, ": ", $e->getMessage(), "\n"; + } +} +?> +--EXPECT-- +one-byte length overflow: PDOException: Firebird database parameter "user" is too long +DPB capacity overflow: PDOException: Firebird database parameter "password" is too long