Skip to content

Commit 2920a26

Browse files
NattyNarwhalcmb69
authored andcommitted
Quote when adding to connection string in (PDO_)ODBC
Because the UID= and PWD= values are appended to the SQLDriverConnect case when credentials are passed, we have to append them to the string in case users are relying on this behaviour. However, they must be quoted, or the arguments will be invalid (or possibly more injected). This means users had to quote arguments or append credentials to the raw connection string themselves. It seems that ODBC quoting rules are consistent enough (and that Microsoft trusts them enough to encode into the .NET BCL) that we can actually check if the string is already quoted (in case a user is already quoting because of this not being fixed), and if not, apply the appropriate ODBC quoting rules. This is because the code exists in main/, and are shared between both ODBC extensions, so it doesn't make sense for it to only exist in one or the other. There may be a better spot for it. Closes phpGH-8307.
1 parent 1400aa4 commit 2920a26

14 files changed

+410
-16
lines changed

NEWS

+4
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,13 @@ PHP NEWS
4242
. Support for building against Oracle Client libraries 10.1 and 10.2 has been
4343
dropped. Oracle Client libraries 11.2 or newer are now required.
4444

45+
- ODBC:
46+
. Automatically quote username and password when needed. (Calvin Buckley)
47+
4548
- PDO_ODBC:
4649
. Fixed bug #80909 (crash with persistent connections in PDO_ODBC). (Calvin
4750
Buckley)
51+
. Automatically quote username and password when needed. (Calvin Buckley)
4852

4953
- Reflection:
5054
. Added ReflectionFunction::isAnonymous(). (Nicolas Grekas)

UPGRADING

+20
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,19 @@ PHP 8.2 UPGRADE NOTES
2525
. DateTimeImmutable::createFromMutable() now has a tentative return type of static,
2626
previously it was DateTimeImmutable.
2727

28+
- ODBC:
29+
. The ODBC extension now escapes the username and password for the case when
30+
both a connection string and username/password are passed, and the string
31+
must be appended to. Before, user values containing values needing escaping
32+
could have created a malformed connection string, or injected values from
33+
user-provided data. The escaping rules should be identical to the .NET BCL
34+
DbConnectionOptions behaviour.
35+
36+
- PDO_ODBC:
37+
. The PDO_ODBC extension also escapes the username and password when a
38+
connection string is passed. See the change to the ODBC extension for
39+
further details.
40+
2841
- Standard:
2942
. strtolower() and strtoupper() are no longer locale-sensitive. They now
3043
perform ASCII case conversion, as if the locale were "C". Use
@@ -70,6 +83,13 @@ PHP 8.2 UPGRADE NOTES
7083
round-trips between PHP and Oracle Database when fetching LOBS. This is
7184
usable with Oracle Database 12.2 or later.
7285

86+
- ODBC:
87+
. Added odbc_connection_string_is_quoted, odbc_connection_string_should_quote,
88+
and odbc_connection_string_quote. These are primarily used behind the scenes
89+
in the ODBC and PDO_ODBC extensions, but is exposed to userland for easier
90+
unit testing, and for user applications and libraries to perform quoting
91+
themselves.
92+
7393
- PCRE:
7494
. Added support for the "n" (NO_AUTO_CAPTURE) modifier, which makes simple
7595
`(xyz)` groups non-capturing. Only named groups like `(?<name>xyz)` are

configure.ac

+1-1
Original file line numberDiff line numberDiff line change
@@ -1614,7 +1614,7 @@ PHP_ADD_SOURCES(main, main.c snprintf.c spprintf.c \
16141614
php_ini_builder.c \
16151615
php_ini.c SAPI.c rfc1867.c php_content_types.c strlcpy.c \
16161616
strlcat.c explicit_bzero.c reentrancy.c php_variables.c php_ticks.c \
1617-
network.c php_open_temporary_file.c \
1617+
network.c php_open_temporary_file.c php_odbc_utils.c \
16181618
output.c getopt.c php_syslog.c, -DZEND_ENABLE_STATIC_TSRMLS_CACHE=1)
16191619

16201620
PHP_ADD_SOURCES_X(main, fastcgi.c, -DZEND_ENABLE_STATIC_TSRMLS_CACHE=1, PHP_FASTCGI_OBJS, no)

ext/odbc/config.m4

+1-1
Original file line numberDiff line numberDiff line change
@@ -461,7 +461,7 @@ if test -n "$ODBC_TYPE"; then
461461
PHP_SUBST_OLD(ODBC_LFLAGS)
462462
PHP_SUBST_OLD(ODBC_TYPE)
463463

464-
PHP_NEW_EXTENSION(odbc, php_odbc.c, $ext_shared,, [$ODBC_CFLAGS -DZEND_ENABLE_STATIC_TSRMLS_CACHE=1])
464+
PHP_NEW_EXTENSION(odbc, php_odbc.c odbc_utils.c, $ext_shared,, [$ODBC_CFLAGS -DZEND_ENABLE_STATIC_TSRMLS_CACHE=1])
465465
else
466466
AC_MSG_CHECKING([for any ODBC driver support])
467467
AC_MSG_RESULT(no)

ext/odbc/config.w32

+1-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ if (PHP_ODBC == "yes") {
77
if (CHECK_LIB("odbc32.lib", "odbc") && CHECK_LIB("odbccp32.lib", "odbc")
88
&& CHECK_HEADER_ADD_INCLUDE("sql.h", "CFLAGS_ODBC")
99
&& CHECK_HEADER_ADD_INCLUDE("sqlext.h", "CFLAGS_ODBC")) {
10-
EXTENSION("odbc", "php_odbc.c", PHP_ODBC_SHARED, "/DZEND_ENABLE_STATIC_TSRMLS_CACHE=1");
10+
EXTENSION("odbc", "php_odbc.c odbc_utils.c", PHP_ODBC_SHARED, "/DZEND_ENABLE_STATIC_TSRMLS_CACHE=1");
1111
AC_DEFINE("HAVE_UODBC", 1, "ODBC support");
1212
if ("no" == PHP_ODBCVER) {
1313
AC_DEFINE("ODBCVER", "0x0350", "The highest supported ODBC version", false);

ext/odbc/odbc.stub.php

+8
Original file line numberDiff line numberDiff line change
@@ -197,3 +197,11 @@ function odbc_tableprivileges($odbc, ?string $catalog, string $schema, string $t
197197
*/
198198
function odbc_columnprivileges($odbc, ?string $catalog, string $schema, string $table, string $column) {}
199199
#endif
200+
201+
/* odbc_utils.c */
202+
203+
function odbc_connection_string_is_quoted(string $str): bool {}
204+
205+
function odbc_connection_string_should_quote(string $str): bool {}
206+
207+
function odbc_connection_string_quote(string $str): string {}

ext/odbc/odbc_arginfo.h

+17-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/* This is a generated file, edit the .stub.php file instead.
2-
* Stub hash: 27a50ba79ed632721ee458527ef543e4b44ee897 */
2+
* Stub hash: 298e48377c2d18c532d91a9ed97886b49a64c096 */
33

44
ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_odbc_close_all, 0, 0, IS_VOID, 0)
55
ZEND_END_ARG_INFO()
@@ -245,6 +245,16 @@ ZEND_BEGIN_ARG_INFO_EX(arginfo_odbc_columnprivileges, 0, 0, 5)
245245
ZEND_END_ARG_INFO()
246246
#endif
247247

248+
ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_odbc_connection_string_is_quoted, 0, 1, _IS_BOOL, 0)
249+
ZEND_ARG_TYPE_INFO(0, str, IS_STRING, 0)
250+
ZEND_END_ARG_INFO()
251+
252+
#define arginfo_odbc_connection_string_should_quote arginfo_odbc_connection_string_is_quoted
253+
254+
ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_odbc_connection_string_quote, 0, 1, IS_STRING, 0)
255+
ZEND_ARG_TYPE_INFO(0, str, IS_STRING, 0)
256+
ZEND_END_ARG_INFO()
257+
248258

249259
ZEND_FUNCTION(odbc_close_all);
250260
ZEND_FUNCTION(odbc_binmode);
@@ -307,6 +317,9 @@ ZEND_FUNCTION(odbc_tableprivileges);
307317
#if !defined(HAVE_DBMAKER) && !defined(HAVE_SOLID) && !defined(HAVE_SOLID_30) &&!defined(HAVE_SOLID_35)
308318
ZEND_FUNCTION(odbc_columnprivileges);
309319
#endif
320+
ZEND_FUNCTION(odbc_connection_string_is_quoted);
321+
ZEND_FUNCTION(odbc_connection_string_should_quote);
322+
ZEND_FUNCTION(odbc_connection_string_quote);
310323

311324

312325
static const zend_function_entry ext_functions[] = {
@@ -373,5 +386,8 @@ static const zend_function_entry ext_functions[] = {
373386
#if !defined(HAVE_DBMAKER) && !defined(HAVE_SOLID) && !defined(HAVE_SOLID_30) &&!defined(HAVE_SOLID_35)
374387
ZEND_FE(odbc_columnprivileges, arginfo_odbc_columnprivileges)
375388
#endif
389+
ZEND_FE(odbc_connection_string_is_quoted, arginfo_odbc_connection_string_is_quoted)
390+
ZEND_FE(odbc_connection_string_should_quote, arginfo_odbc_connection_string_should_quote)
391+
ZEND_FE(odbc_connection_string_quote, arginfo_odbc_connection_string_quote)
376392
ZEND_FE_END
377393
};

ext/odbc/odbc_utils.c

+68
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
/*
2+
+----------------------------------------------------------------------+
3+
| Copyright (c) The PHP Group |
4+
+----------------------------------------------------------------------+
5+
| This source file is subject to version 3.01 of the PHP license, |
6+
| that is bundled with this package in the file LICENSE, and is |
7+
| available through the world-wide-web at the following url: |
8+
| https://www.php.net/license/3_01.txt |
9+
| If you did not receive a copy of the PHP license and are unable to |
10+
| obtain it through the world-wide-web, please send a note to |
11+
| [email protected] so we can mail you a copy immediately. |
12+
+----------------------------------------------------------------------+
13+
| Author: Calvin Buckley <[email protected]> |
14+
+----------------------------------------------------------------------+
15+
*/
16+
17+
#include "php.h"
18+
#include "php_odbc_utils.h"
19+
20+
/*
21+
* Utility functions for dealing with ODBC connection strings and other common
22+
* functionality.
23+
*
24+
* While useful for PDO_ODBC too, this lives in ext/odbc because there isn't a
25+
* better place for it.
26+
*/
27+
28+
PHP_FUNCTION(odbc_connection_string_is_quoted)
29+
{
30+
zend_string *str;
31+
32+
ZEND_PARSE_PARAMETERS_START(1, 1)
33+
Z_PARAM_STR(str)
34+
ZEND_PARSE_PARAMETERS_END();
35+
36+
bool is_quoted = php_odbc_connstr_is_quoted(ZSTR_VAL(str));
37+
38+
RETURN_BOOL(is_quoted);
39+
}
40+
41+
PHP_FUNCTION(odbc_connection_string_should_quote)
42+
{
43+
zend_string *str;
44+
45+
ZEND_PARSE_PARAMETERS_START(1, 1)
46+
Z_PARAM_STR(str)
47+
ZEND_PARSE_PARAMETERS_END();
48+
49+
bool should_quote = php_odbc_connstr_should_quote(ZSTR_VAL(str));
50+
51+
RETURN_BOOL(should_quote);
52+
}
53+
54+
PHP_FUNCTION(odbc_connection_string_quote)
55+
{
56+
zend_string *str;
57+
58+
ZEND_PARSE_PARAMETERS_START(1, 1)
59+
Z_PARAM_STR(str)
60+
ZEND_PARSE_PARAMETERS_END();
61+
62+
size_t new_size = php_odbc_connstr_estimate_quote_length(ZSTR_VAL(str));
63+
zend_string *new_string = zend_string_alloc(new_size, 0);
64+
php_odbc_connstr_quote(ZSTR_VAL(new_string), ZSTR_VAL(str), new_size);
65+
/* reset length */
66+
ZSTR_LEN(new_string) = strlen(ZSTR_VAL(new_string));
67+
RETURN_STR(new_string);
68+
}

ext/odbc/php_odbc.c

+35-2
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,9 @@
3434
#include "php_globals.h"
3535
#include "odbc_arginfo.h"
3636

37+
/* actually lives in main/ */
38+
#include "php_odbc_utils.h"
39+
3740
#ifdef HAVE_UODBC
3841

3942
#include <fcntl.h>
@@ -2169,8 +2172,38 @@ int odbc_sqlconnect(odbc_connection **conn, char *db, char *uid, char *pwd, int
21692172

21702173
if (strstr((char*)db, ";")) {
21712174
direct = 1;
2172-
if (uid && !strstr ((char*)db, "uid") && !strstr((char*)db, "UID")) {
2173-
spprintf(&ldb, 0, "%s;UID=%s;PWD=%s", db, uid, pwd);
2175+
/* Force UID and PWD to be set in the DSN */
2176+
bool is_uid_set = uid && *uid
2177+
&& !strstr(db, "uid=")
2178+
&& !strstr(db, "UID=");
2179+
bool is_pwd_set = pwd && *pwd
2180+
&& !strstr(db, "pwd=")
2181+
&& !strstr(db, "PWD=");
2182+
if (is_uid_set && is_pwd_set) {
2183+
char *uid_quoted = NULL, *pwd_quoted = NULL;
2184+
bool should_quote_uid = !php_odbc_connstr_is_quoted(uid) && php_odbc_connstr_should_quote(uid);
2185+
bool should_quote_pwd = !php_odbc_connstr_is_quoted(pwd) && php_odbc_connstr_should_quote(pwd);
2186+
if (should_quote_uid) {
2187+
size_t estimated_length = php_odbc_connstr_estimate_quote_length(uid);
2188+
uid_quoted = emalloc(estimated_length);
2189+
php_odbc_connstr_quote(uid_quoted, uid, estimated_length);
2190+
} else {
2191+
uid_quoted = uid;
2192+
}
2193+
if (should_quote_pwd) {
2194+
size_t estimated_length = php_odbc_connstr_estimate_quote_length(pwd);
2195+
pwd_quoted = emalloc(estimated_length);
2196+
php_odbc_connstr_quote(pwd_quoted, pwd, estimated_length);
2197+
} else {
2198+
pwd_quoted = pwd;
2199+
}
2200+
spprintf(&ldb, 0, "%s;UID=%s;PWD=%s", db, uid_quoted, pwd_quoted);
2201+
if (uid_quoted && should_quote_uid) {
2202+
efree(uid_quoted);
2203+
}
2204+
if (pwd_quoted && should_quote_pwd) {
2205+
efree(pwd_quoted);
2206+
}
21742207
} else {
21752208
ldb_len = strlen(db)+1;
21762209
ldb = (char*) emalloc(ldb_len);

ext/odbc/tests/odbc_utils.phpt

+81
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
--TEST--
2+
Test common ODBC string functionality
3+
--EXTENSIONS--
4+
odbc
5+
--FILE--
6+
<?php
7+
8+
// 1. No, it's not quoted.
9+
// 2. Yes, it should be quoted because of the special character in the middle.
10+
$with_end_curly1 = "foo}bar";
11+
// 1. No, the unescaped special character in the middle breaks what would be quoted.
12+
// 2. Yes, it should be quoted because of the special character in the middle.
13+
// Note that should_quote doesn't care about if the string is already quoted.
14+
// That's why you should check if it is quoted first.
15+
$with_end_curly2 = "{foo}bar}";
16+
// 1. Yes, the special characters are escaped, so it's quoted.
17+
// 2. See $with_end_curly2; should_quote doesn't care about if the string is already quoted.
18+
$with_end_curly3 = "{foo}}bar}";
19+
// 1. No, it's not quoted.
20+
// 2. It doesn't need to be quoted because of no s
21+
$with_no_end_curly1 = "foobar";
22+
// 1. Yes, it is quoted and any characters are properly escaped.
23+
// 2. See $with_end_curly2.
24+
$with_no_end_curly2 = "{foobar}";
25+
26+
echo "# Is quoted?\n";
27+
echo "With end curly brace 1: ";
28+
var_dump(odbc_connection_string_is_quoted($with_end_curly1));
29+
echo "With end curly brace 2: ";
30+
var_dump(odbc_connection_string_is_quoted($with_end_curly2));
31+
echo "With end curly brace 3: ";
32+
var_dump(odbc_connection_string_is_quoted($with_end_curly3));
33+
echo "Without end curly brace 1: ";
34+
var_dump(odbc_connection_string_is_quoted($with_no_end_curly1));
35+
echo "Without end curly brace 2: ";
36+
var_dump(odbc_connection_string_is_quoted($with_no_end_curly2));
37+
38+
echo "# Should quote?\n";
39+
echo "With end curly brace 1: ";
40+
var_dump(odbc_connection_string_should_quote($with_end_curly1));
41+
echo "With end curly brace 2: ";
42+
var_dump(odbc_connection_string_should_quote($with_end_curly2));
43+
echo "With end curly brace 3: ";
44+
var_dump(odbc_connection_string_should_quote($with_end_curly3));
45+
echo "Without end curly brace 1: ";
46+
var_dump(odbc_connection_string_should_quote($with_no_end_curly1));
47+
echo "Without end curly brace 2: ";
48+
var_dump(odbc_connection_string_should_quote($with_no_end_curly2));
49+
50+
echo "# Quote?\n";
51+
echo "With end curly brace 1: ";
52+
var_dump(odbc_connection_string_quote($with_end_curly1));
53+
echo "With end curly brace 2: ";
54+
var_dump(odbc_connection_string_quote($with_end_curly2));
55+
echo "With end curly brace 3: ";
56+
var_dump(odbc_connection_string_quote($with_end_curly3));
57+
echo "Without end curly brace 1: ";
58+
var_dump(odbc_connection_string_quote($with_no_end_curly1));
59+
echo "Without end curly brace 2: ";
60+
var_dump(odbc_connection_string_quote($with_no_end_curly2));
61+
62+
?>
63+
--EXPECTF--
64+
# Is quoted?
65+
With end curly brace 1: bool(false)
66+
With end curly brace 2: bool(false)
67+
With end curly brace 3: bool(true)
68+
Without end curly brace 1: bool(false)
69+
Without end curly brace 2: bool(true)
70+
# Should quote?
71+
With end curly brace 1: bool(true)
72+
With end curly brace 2: bool(true)
73+
With end curly brace 3: bool(true)
74+
Without end curly brace 1: bool(false)
75+
Without end curly brace 2: bool(true)
76+
# Quote?
77+
With end curly brace 1: string(10) "{foo}}bar}"
78+
With end curly brace 2: string(13) "{{foo}}bar}}}"
79+
With end curly brace 3: string(15) "{{foo}}}}bar}}}"
80+
Without end curly brace 1: string(8) "{foobar}"
81+
Without end curly brace 2: string(11) "{{foobar}}}"

ext/pdo_odbc/odbc_driver.c

+34-9
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323
#include "ext/standard/info.h"
2424
#include "pdo/php_pdo.h"
2525
#include "pdo/php_pdo_driver.h"
26+
/* this file actually lives in main/ */
27+
#include "php_odbc_utils.h"
2628
#include "php_pdo_odbc.h"
2729
#include "php_pdo_odbc_int.h"
2830
#include "zend_exceptions.h"
@@ -485,20 +487,43 @@ static int pdo_odbc_handle_factory(pdo_dbh_t *dbh, zval *driver_options) /* {{{
485487
use_direct = 1;
486488

487489
/* Force UID and PWD to be set in the DSN */
488-
if (dbh->username && *dbh->username && !strstr(dbh->data_source, "uid")
489-
&& !strstr(dbh->data_source, "UID")) {
490-
/* XXX: Do we check if password is null? */
490+
bool is_uid_set = dbh->username && *dbh->username
491+
&& !strstr(dbh->data_source, "uid=")
492+
&& !strstr(dbh->data_source, "UID=");
493+
bool is_pwd_set = dbh->password && *dbh->password
494+
&& !strstr(dbh->data_source, "pwd=")
495+
&& !strstr(dbh->data_source, "PWD=");
496+
if (is_uid_set && is_pwd_set) {
497+
char *uid = NULL, *pwd = NULL;
498+
bool should_quote_uid = !php_odbc_connstr_is_quoted(dbh->username) && php_odbc_connstr_should_quote(dbh->username);
499+
bool should_quote_pwd = !php_odbc_connstr_is_quoted(dbh->password) && php_odbc_connstr_should_quote(dbh->password);
500+
if (should_quote_uid) {
501+
size_t estimated_length = php_odbc_connstr_estimate_quote_length(dbh->username);
502+
uid = emalloc(estimated_length);
503+
php_odbc_connstr_quote(uid, dbh->username, estimated_length);
504+
} else {
505+
uid = dbh->username;
506+
}
507+
if (should_quote_pwd) {
508+
size_t estimated_length = php_odbc_connstr_estimate_quote_length(dbh->password);
509+
pwd = emalloc(estimated_length);
510+
php_odbc_connstr_quote(pwd, dbh->password, estimated_length);
511+
} else {
512+
pwd = dbh->password;
513+
}
491514
size_t new_dsn_size = strlen(dbh->data_source)
492-
+ strlen(dbh->username) + strlen(dbh->password)
515+
+ strlen(uid) + strlen(pwd)
493516
+ strlen(";UID=;PWD=") + 1;
494517
char *dsn = pemalloc(new_dsn_size, dbh->is_persistent);
495-
if (dsn == NULL) {
496-
/* XXX: Do we inform the caller? */
497-
goto fail;
498-
}
499-
snprintf(dsn, new_dsn_size, "%s;UID=%s;PWD=%s", dbh->data_source, dbh->username, dbh->password);
518+
snprintf(dsn, new_dsn_size, "%s;UID=%s;PWD=%s", dbh->data_source, uid, pwd);
500519
pefree((char*)dbh->data_source, dbh->is_persistent);
501520
dbh->data_source = dsn;
521+
if (uid && should_quote_uid) {
522+
efree(uid);
523+
}
524+
if (pwd && should_quote_pwd) {
525+
efree(pwd);
526+
}
502527
}
503528

504529
rc = SQLDriverConnect(H->dbc, NULL, (SQLCHAR *) dbh->data_source, strlen(dbh->data_source),

0 commit comments

Comments
 (0)