[php-src] Issue #24736: ext/odbc, ext/pdo_odbc: php_odbc_connstr_is_quoted() treats a value with no closing brace as "already quoted," skipping
UID/PWD escaping

Issue: https://github.com/php/php-src/issues/24736 Author: kunwaratulhax0r ### 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: ```c 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: ```c 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 <?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: ```c 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.