Hacktoberfest 2026: los issues que los mantenedores marcaron para octubre, abiertos y aptos para principiantes. Explorar issues de Hacktoberfest

ext/odbc, ext/pdo_odbc: php_odbc_connstr_is_quoted() treats a value with no closing brace as "already quoted," skipping UID/PWD escaping

Abierto Apto para principiantes
#24,736 0 comentarios 0 reacciones 0 asignados Ver en GitHub

Los mantenedores suelen responder en 1 día

@lazerg ya está trabajando en esto.

Desde el 26/10/2026.

  • #24737 de @lazerg — abierto

Evaluación

Dificultad
2/5
Tiempo estimado
1-3 horas
Aptitud para principiantes
76/100
Tipo de issue
Error
Claridad
Bien especificado
Estado de actividad
Activo
Stack tecnológico
c, php
Área
backend, security

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

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Más de php/php-src

Todos los issues de php/php-src

Issues similares

Más issues de C

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.