ext/odbc, ext/pdo_odbc: php_odbc_connstr_is_quoted() treats a value with no closing brace as "already quoted," skipping UID/PWD escaping
Los mantenedores suelen responder en 1 día
Evaluación
- Dificultad
- 2/5
- Tiempo estimado
- 1-3 horas
- Aptitud para principiantes
- 76/100
Línea de trabajo
Empieza en main/php_odbc_utils.c e inspecciona php_odbc_connstr_is_quoted(), luego revisa el reproductor del issue. Comprueba las pruebas existentes de los auxiliares de ODBC o añade cobertura para una llave de apertura sin una llave de cierre; se considera terminado cuando esa entrada no se considera entrecomillada y puede ejecutarse la comprobación de escape. El issue no informa de comentarios ni de pull requests enlazados.
Escrito por el modelo de indexación a partir del texto del issue.
Descripción
Description
php_odbc_connstr_is_quoted() (main/php_odbc_utils.c) is the gate that decides whether odbc_connect()/odbc_pconnect()/PDO's odbc driver should apply php_odbc_connstr_quote() before appending $username/$password to the connection string:
PHPAPI bool php_odbc_connstr_is_quoted(const char *str)
{
if (str[0] != '{') {
return false;
}
size_t length = strlen(str);
for (size_t i = 0; i < length; i++) {
if (str[i] == '}' && str[i + 1] == '}') {
i++;
} else if (str[i] == '}' && str[i + 1] != '\0') {
return false;
}
}
return true;
}
It checks that every } present is doubled-or-terminal, but never checks that a } actually exists. A string that starts with { and contains zero } characters anywhere vacuously satisfies the loop and returns true ("already safely quoted") even though it has no closing brace at all.
Both call sites (ext/odbc/php_odbc.c:1985/:2000, ext/pdo_odbc/odbc_driver.c:551/:568) use this as:
should_quote_uid = !php_odbc_connstr_is_quoted(uid) && php_odbc_connstr_should_quote(uid);
if (should_quote_uid) {
/* apply php_odbc_connstr_quote() */
} else {
uid_quoted = uid; /* raw value used as-is */
}
so a $username/$password value crafted to start with { and contain no } skips the escaping added for GH-8307/GH-8300 entirely, and gets appended to the connection string raw.
Minimal reproducer — no ODBC driver or database needed, the helpers are exposed directly:
<?php
$payload = "{;Trusted_Connection=yes;UID=x";
var_dump(odbc_connection_string_is_quoted($payload));
var_dump(odbc_connection_string_should_quote($payload));
var_dump(!odbc_connection_string_is_quoted($payload) && odbc_connection_string_should_quote($payload));
Resulted in this output:
bool(true)
bool(true)
bool(false)
But I expected this output instead:
bool(false)
bool(true)
bool(true)
i.e. a string with an opening { and no matching } should NOT be treated as already-quoted, so the escaping gate (the third line, mirroring the real connect code's own condition) should fire instead of being skipped.
Confirmed present, byte-identical, on PHP-8.3, PHP-8.4, PHP-8.5, and master — unchanged since the function was introduced in 2022 (2920a266364aef84df82cbb2b01b573705fa57ec, GH-8307) and untouched by the two later hardening commits to the same file (0d9ff00394d9447992bb66ccb1cef3edf70576bd, 20003bec2b9f9db87cea59fb395bd2e736d1a8a0).
Suggested fix: track whether an actual (non-doubled, terminal) closing } was found during the scan, and return that instead of unconditionally true:
PHPAPI bool php_odbc_connstr_is_quoted(const char *str)
{
if (str[0] != '{') {
return false;
}
size_t length = strlen(str);
bool found_closing_brace = false;
for (size_t i = 0; i < length; i++) {
if (str[i] == '}' && str[i + 1] == '}') {
i++;
} else if (str[i] == '}' && str[i + 1] != '\0') {
return false;
} else if (str[i] == '}') {
found_closing_brace = true;
}
}
return found_closing_brace;
}
On real-world impact: I tried to turn this into an actual connection-string injection against two real ODBC driver stacks via odbc_connect($dsn, $payload, $pwd) — psqlODBC against a real PostgreSQL server (trying to inject ConnSettings=, which runs arbitrary SQL on connect), and FreeTDS (trying to redirect Server=/Port= to a listener I controlled). Both failed safely: psqlODBC's own parser rejected the malformed string outright ("Connection string parse error"), and FreeTDS never honored the injected Server=/Port= override. Both results look like the underlying driver manager (unixODBC) does its own independent brace-aware parsing, which would make this bug redundant-but-still-wrong rather than directly exploitable on at least those two stacks. I don't have access to test Windows' ODBC Driver Manager, iODBC, or proprietary drivers (Oracle, DB2, etc.), so I can't rule out this being exploitable there. Reporting it as a logic bug in the escaping helper regardless, since relying on an unproven, undocumented safety net in a separate library isn't a sound substitute for the helper doing its own job correctly.
PHP Version
PHP 8.7.0-dev (cli) (built: Oct 26 2026 09:57:01) (NTS DEBUG)
Copyright © The PHP Group and Contributors
Zend Engine v4.7.0-dev, Copyright © Zend by Perforce
with Zend OPcache v8.7.0-dev, Copyright ©, by Zend by Perforce
Also confirmed present on the PHP-8.3, PHP-8.4, and PHP-8.5 branches via direct content check.
Operating System
Ubuntu 24.04 (Docker), but this is pure string-handling logic, not OS-specific.
- Lenguaje dominante
- C
- Estrellas
- 40.7k
- Forks
- 8.3k
- Merge medio
- 2 d 17 h
- PR fusionados (30 d)
- 159
Preparar el entorno
- Sin Dockerfile ni archivo de Docker Compose
- Sin plantilla de pull request
- Leer la guía de contribución
Primeros pasos
- Lee el issue completo y luego la guía de contribución del proyecto.
- Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
- Haz un fork del repositorio y trabaja en una rama.
- Abre un pull request que haga referencia al número del issue.
Más de php/php-src
-
Bug Status: Needs Triage
Dificultad 2/5 1-3 horas Aptitud para principiantes 78/100
Los mantenedores suelen responder en 1 día
-
zend_set_local_var() overwrites existing compiled variable (CV) zval without calling zval_ptr_dtor()Posiblemente ocupada Un pull request vinculado a esta issue está abierto o ya se fusionó. AbiertoBug Status: Needs Triage
Dificultad 2/5 1-3 horas Aptitud para principiantes 82/100
Los mantenedores suelen responder en 1 día
-
Dificultad 2/5 1-3 horas Aptitud para principiantes 72/100
Los mantenedores suelen responder en 1 día
-
Case-insensitive substr_compare, ksort, krsort are still locale-dependentPosiblemente ocupada @lazerg la tomó hace 5 días. AbiertoBug Extension: standard Status: Verified
Dificultad 2/5 1-3 horas Aptitud para principiantes 73/100
php/php-src#24552 · 1 comentario ·
Los mantenedores suelen responder en 1 día
-
master: part of the mbstring bailout fix got lost in a mergePosiblemente ocupada @lazerg la tomó hace 5 días. Abierto
Dificultad 2/5 1-3 horas Aptitud para principiantes 78/100
Los mantenedores suelen responder en 1 día
Todos los issues de php/php-src
Issues similares
-
bug intrinsic medium priority rt_lib
Dificultad 2/5 1-3 horas Aptitud para principiantes 75/100
Los mantenedores suelen responder en 1 día
-
[CLI] Clarify distinction between 'capture camera' and 'capture screen' in help text and outputAbierto
Dificultad 2/5 1-3 horas Aptitud para principiantes 68/100
espressif/esp-vision#18 ·
-
Dificultad 2/5 1-3 horas Aptitud para principiantes 68/100
sumatrapdfreader/sumatrapdf#6417 · 3 comentarios ·
-
[BUG] Cython cache evicts most recent instead of oldest entry on cleanupPosiblemente ocupada @scoder la tomó hoy. AbiertoBuild System defect
Dificultad 2/5 1-3 horas Aptitud para principiantes 75/100
Los mantenedores suelen responder en 2 días
-
Dificultad 2/5 1-3 horas Aptitud para principiantes 61/100
HexaCluster/credcheck#94 ·