Hacktoberfest 2026: die Issues, die Maintainer für den Oktober markiert haben – offen und einsteigerfreundlich. Hacktoberfest-Issues durchsuchen

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

Geschlossen Anfängerfreundlich
  • Stimmt etwas nicht?
#24,736 0 Kommentare 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen

Maintainer antworten meist innerhalb von 1 Tag

Dieses Issue hat noch niemand übernommen.

Bewertung

Schwierigkeit
2/5
Geschätzter Aufwand
1-3 Stunden
Anfängerfreundlichkeit
76/100
Issue-Typ
Bug
Klarheit
Klar beschrieben
Aktivitätsstatus
Aktiv
Tech-Stack
c, php
Bereich
backend, security

Rechercherichtung

Beginne in main/php_odbc_utils.c und untersuche php_odbc_connstr_is_quoted(), dann sieh dir den Reproducer im Issue an. Prüfe die vorhandenen ODBC-Hilfstests oder ergänze Tests für eine öffnende geschweifte Klammer ohne schließende Klammer; fertig ist die Änderung, wenn diese Eingabe nicht als zitiert gilt und die Escaping-Prüfung ausgeführt werden kann. Im Issue werden keine Kommentare oder verlinkten Pull Requests gemeldet.

Vom Indexierungsmodell aus dem Issue-Text verfasst.

Beschreibung

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.

Vorherrschende Sprache
C
Sterne
40.7k
Forks
8.3k
Ø Merge
2 T. 18 Std.
Gemergte PRs (30 T.)
159

Entwicklungsumgebung

Erste Schritte

  1. Lesen Sie das ganze Issue und danach den Beitragsleitfaden des Projekts.
  2. Schreiben Sie ins Issue, dass Sie es übernehmen — das erspart doppelte Arbeit.
  3. Forken Sie das Repository und arbeiten Sie in einem Branch.
  4. Öffnen Sie einen Pull Request, der die Issue-Nummer nennt.

Mehr aus php/php-src

Alle Issues in php/php-src

Ähnliche Issues

Weitere Issues zu C

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.