Skip to content

Commit f718684

Browse files
author
Yasuo Ohgaki
committed
Fixed bug #62978. pg_select()/etc may allow SQL injection when table name is user parameter, users are able to control table names.
1 parent a93a462 commit f718684

15 files changed

+184
-62
lines changed

‎ext/pgsql/pgsql.c

+150-28
Original file line numberDiff line numberDiff line change
@@ -914,6 +914,60 @@ static void _free_result(zend_rsrc_list_entry *rsrc TSRMLS_DC)
914914
}
915915
/* }}} */
916916

917+
918+
static int _php_pgsql_detect_identifier_escape(const char *identifier, size_t len)
919+
{
920+
size_t i;
921+
922+
/* Handle edge case. Cannot be a escaped string */
923+
if (len <= 2) {
924+
return FAILURE;
925+
}
926+
/* Detect double qoutes */
927+
if (identifier[0] == '"' && identifier[len-1] == '"') {
928+
/* Detect wrong format of " inside of escaped string */
929+
for (i = 1; i < len-1; i++) {
930+
if (identifier[i] == '"' && (identifier[++i] != '"' || i == len-1)) {
931+
return FAILURE;
932+
}
933+
}
934+
} else {
935+
return FAILURE;
936+
}
937+
/* Escaped properly */
938+
return SUCCESS;
939+
}
940+
941+
#if !HAVE_PQESCAPELITERAL
942+
/* {{{ _php_pgsql_escape_identifier
943+
* Since PQescapeIdentifier() is unavailable (PostgreSQL 9.0 <), idenfifers
944+
* should be escaped by pgsql module.
945+
* Note: this function does not care for encoding. Therefore users should not
946+
* use this with SJIS/BIG5 etc. (i.e. Encoding base injection may possible with
947+
* before PostgreSQL 9.0)
948+
*/
949+
static char *_php_pgsql_escape_identifier(const char *field, size_t field_len)
950+
{
951+
ulong field_escaped_len = field_len*2 + 3;
952+
ulong i, j = 0;
953+
char *field_escaped;
954+
955+
field_escaped = (char *)malloc(field_escaped_len);
956+
field_escaped[j++] = '"';
957+
for (i = 0; i < field_len; i++) {
958+
if (field[i] == '"') {
959+
field_escaped[j++] = '"';
960+
field_escaped[j++] = '"';
961+
} else {
962+
field_escaped[j++] = field[i];
963+
}
964+
}
965+
field_escaped[j++] = '"';
966+
field_escaped[j] = '\0';
967+
return field_escaped;
968+
}
969+
#endif
970+
917971
/* {{{ PHP_INI
918972
*/
919973
PHP_INI_BEGIN()
@@ -5015,8 +5069,9 @@ PHP_PGSQL_API int php_pgsql_meta_data(PGconn *pg_link, const char *table_name, z
50155069
{
50165070
PGresult *pg_result;
50175071
char *src, *tmp_name, *tmp_name2 = NULL;
5072+
char *escaped;
50185073
smart_str querystr = {0};
5019-
int new_len;
5074+
size_t new_len;
50205075
int i, num_rows;
50215076
zval *elem;
50225077

@@ -5038,20 +5093,29 @@ PHP_PGSQL_API int php_pgsql_meta_data(PGconn *pg_link, const char *table_name, z
50385093
"SELECT a.attname, a.attnum, t.typname, a.attlen, a.attnotnull, a.atthasdef, a.attndims, t.typtype = 'e' "
50395094
"FROM pg_class as c, pg_attribute a, pg_type t, pg_namespace n "
50405095
"WHERE a.attnum > 0 AND a.attrelid = c.oid AND c.relname = '");
5041-
tmp_name2 = php_addslashes(tmp_name2, strlen(tmp_name2), &new_len, 0 TSRMLS_CC);
5042-
smart_str_appendl(&querystr, tmp_name2, new_len);
5043-
5096+
escaped = (char *)safe_emalloc(strlen(tmp_name2), 2, 1);
5097+
#if HAVE_PQESCAPE_CONN
5098+
new_len = PQescapeStringConn(pg_link, escaped, tmp_name2, strlen(tmp_name2), NULL);
5099+
#else
5100+
new_len = PQescapeString(escaped, tmp_name2, strlen(tmp_name2));
5101+
#endif
5102+
smart_str_appends(&querystr, escaped);
5103+
efree(escaped);
5104+
50445105
smart_str_appends(&querystr, "' AND c.relnamespace = n.oid AND n.nspname = '");
5045-
tmp_name = php_addslashes(tmp_name, strlen(tmp_name), &new_len, 0 TSRMLS_CC);
5046-
smart_str_appendl(&querystr, tmp_name, new_len);
5106+
escaped = (char *)safe_emalloc(strlen(tmp_name), 2, 1);
5107+
#if HAVE_PQESCAPE_CONN
5108+
new_len = PQescapeStringConn(pg_link, escaped, tmp_name, strlen(tmp_name), NULL);
5109+
#else
5110+
new_len = PQescapeString(escaped, tmp_name, strlen(tmp_name));
5111+
#endif
5112+
smart_str_appends(&querystr, escaped);
5113+
efree(escaped);
50475114

50485115
smart_str_appends(&querystr, "' AND a.atttypid = t.oid ORDER BY a.attnum;");
50495116
smart_str_0(&querystr);
5050-
5051-
efree(tmp_name2);
5052-
efree(tmp_name);
5053-
efree(src);
5054-
5117+
efree(src);
5118+
50555119
pg_result = PQexec(pg_link, querystr.c);
50565120
if (PQresultStatus(pg_result) != PGRES_TUPLES_OK || (num_rows = PQntuples(pg_result)) == 0) {
50575121
php_error_docref(NULL TSRMLS_CC, E_WARNING, "Table '%s' doesn't exists", table_name);
@@ -5274,6 +5338,7 @@ static int php_pgsql_add_quotes(zval *src, zend_bool should_free TSRMLS_DC)
52745338
assert(Z_TYPE_P(src) == IS_STRING);
52755339
assert(should_free == 1 || should_free == 0);
52765340

5341+
smart_str_appendc(&str, 'E');
52775342
smart_str_appendc(&str, '\'');
52785343
smart_str_appendl(&str, Z_STRVAL_P(src), Z_STRLEN_P(src));
52795344
smart_str_appendc(&str, '\'');
@@ -5314,7 +5379,7 @@ PHP_PGSQL_API int php_pgsql_convert(PGconn *pg_link, const char *table_name, con
53145379
uint field_len = -1;
53155380
ulong num_idx = -1;
53165381
zval *meta, **def, **type, **not_null, **has_default, **is_enum, **val, *new_val;
5317-
int new_len, key_type, err = 0, skip_field;
5382+
int key_type, err = 0, skip_field;
53185383
php_pgsql_data_type data_type;
53195384

53205385
assert(pg_link != NULL);
@@ -5327,6 +5392,8 @@ PHP_PGSQL_API int php_pgsql_convert(PGconn *pg_link, const char *table_name, con
53275392
}
53285393
MAKE_STD_ZVAL(meta);
53295394
array_init(meta);
5395+
5396+
/* table_name is escaped by php_pgsql_meta_data */
53305397
if (php_pgsql_meta_data(pg_link, table_name, meta TSRMLS_CC) == FAILURE) {
53315398
zval_dtor(meta);
53325399
FREE_ZVAL(meta);
@@ -5539,15 +5606,15 @@ PHP_PGSQL_API int php_pgsql_convert(PGconn *pg_link, const char *table_name, con
55395606
}
55405607
else {
55415608
Z_TYPE_P(new_val) = IS_STRING;
5542-
#if HAVE_PQESCAPE
5609+
#if HAVE_PQESCAPE_CONN
55435610
{
55445611
char *tmp;
5545-
tmp = (char *)safe_emalloc(Z_STRLEN_PP(val), 2, 1);
5546-
Z_STRLEN_P(new_val) = (int)PQescapeString(tmp, Z_STRVAL_PP(val), Z_STRLEN_PP(val));
5612+
tmp = (char *)safe_emalloc(Z_STRLEN_PP(val), 2, 1);
5613+
Z_STRLEN_P(new_val) = (int)PQescapeStringConn(pg_link, tmp, Z_STRVAL_PP(val), Z_STRLEN_PP(val), NULL);
55475614
Z_STRVAL_P(new_val) = tmp;
55485615
}
55495616
#else
5550-
Z_STRVAL_P(new_val) = php_addslashes(Z_STRVAL_PP(val), Z_STRLEN_PP(val), &Z_STRLEN_P(new_val), 0 TSRMLS_CC);
5617+
Z_STRVAL_P(new_val) = (int)PQescapeString(Z_STRVAL_PP(val), Z_STRLEN_PP(val), &Z_STRLEN_P(new_val), 0 TSRMLS_CC);
55515618
#endif
55525619
php_pgsql_add_quotes(new_val, 1 TSRMLS_CC);
55535620
}
@@ -5833,6 +5900,7 @@ PHP_PGSQL_API int php_pgsql_convert(PGconn *pg_link, const char *table_name, con
58335900
else {
58345901
unsigned char *tmp;
58355902
size_t to_len;
5903+
smart_str s = {0};
58365904
#ifdef HAVE_PQESCAPE_BYTEA_CONN
58375905
tmp = PQescapeByteaConn(pg_link, Z_STRVAL_PP(val), Z_STRLEN_PP(val), &to_len);
58385906
#else
@@ -5844,7 +5912,11 @@ PHP_PGSQL_API int php_pgsql_convert(PGconn *pg_link, const char *table_name, con
58445912
memcpy(Z_STRVAL_P(new_val), tmp, to_len);
58455913
PQfreemem(tmp);
58465914
php_pgsql_add_quotes(new_val, 1 TSRMLS_CC);
5847-
5915+
smart_str_appendl(&s, Z_STRVAL_P(new_val), Z_STRLEN_P(new_val));
5916+
smart_str_0(&s);
5917+
efree(Z_STRVAL_P(new_val));
5918+
Z_STRVAL_P(new_val) = s.c;
5919+
Z_STRLEN_P(new_val) = s.len;
58485920
}
58495921
break;
58505922

@@ -5929,11 +6001,22 @@ PHP_PGSQL_API int php_pgsql_convert(PGconn *pg_link, const char *table_name, con
59296001
FREE_ZVAL(new_val);
59306002
break; /* break out for() */
59316003
}
6004+
/* If field is NULL and HAS DEFAULT, should be skipped */
59326005
if (!skip_field) {
5933-
/* If field is NULL and HAS DEFAULT, should be skipped */
5934-
field = php_addslashes(field, strlen(field), &new_len, 0 TSRMLS_CC);
5935-
add_assoc_zval(result, field, new_val);
5936-
efree(field);
6006+
char *escaped;
6007+
size_t new_len, field_len = strlen(field);
6008+
6009+
if (_php_pgsql_detect_identifier_escape(field, field_len) == SUCCESS) {
6010+
escaped = strndup(field, field_len);
6011+
} else {
6012+
#if HAVE_PQESCAPELITERAL
6013+
escaped = PQescapeIdentifier(pg_link, field, field_len);
6014+
#else
6015+
escaped = _php_pgsql_escape_identifier(field, field_len);
6016+
#endif
6017+
}
6018+
add_assoc_zval(result, escaped, new_val);
6019+
free(escaped);
59376020
}
59386021
} /* for */
59396022
zval_dtor(meta);
@@ -6008,6 +6091,45 @@ static int do_exec(smart_str *querystr, int expect, PGconn *pg_link, ulong opt T
60086091
return -1;
60096092
}
60106093

6094+
static inline void build_tablename(smart_str *querystr, PGconn *pg_link, const char *table)
6095+
{
6096+
char *table_copy, *escaped, *token, *tmp;
6097+
size_t len;
6098+
6099+
/* schame.table should be "schame"."table" */
6100+
table_copy = estrdup(table);
6101+
token = php_strtok_r(table_copy, ".", &tmp);
6102+
len = strlen(token);
6103+
if (_php_pgsql_detect_identifier_escape(token, len) == SUCCESS) {
6104+
escaped = strndup(token, len);
6105+
} else {
6106+
#if HAVE_PQESCAPELITERAL
6107+
escaped = PQescapeIdentifier(pg_link, token, len);
6108+
#else
6109+
escaped = _php_pgsql_escape_identifier(token, len);
6110+
#endif
6111+
}
6112+
smart_str_appends(querystr, escaped);
6113+
free(escaped);
6114+
if (tmp && *tmp) {
6115+
len = strlen(tmp);
6116+
/* "schema"."table" format */
6117+
if (_php_pgsql_detect_identifier_escape(tmp, len) == SUCCESS) {
6118+
escaped = strndup(tmp, len);
6119+
} else {
6120+
#if HAVE_PQESCAPELITERAL
6121+
escaped = PQescapeIdentifier(pg_link, tmp, len);
6122+
#else
6123+
escaped = _php_pgsql_escape_identifier(tmp, len);
6124+
#endif
6125+
}
6126+
smart_str_appendc(querystr, '.');
6127+
smart_str_appends(querystr, escaped);
6128+
free(escaped);
6129+
}
6130+
efree(table_copy);
6131+
}
6132+
60116133
/* {{{ php_pgsql_insert
60126134
*/
60136135
PHP_PGSQL_API int php_pgsql_insert(PGconn *pg_link, const char *table, zval *var_array, ulong opt, char **sql TSRMLS_DC)
@@ -6027,7 +6149,7 @@ PHP_PGSQL_API int php_pgsql_insert(PGconn *pg_link, const char *table, zval *var
60276149

60286150
if (zend_hash_num_elements(Z_ARRVAL_P(var_array)) == 0) {
60296151
smart_str_appends(&querystr, "INSERT INTO ");
6030-
smart_str_appends(&querystr, table);
6152+
build_tablename(&querystr, pg_link, table);
60316153
smart_str_appends(&querystr, " DEFAULT VALUES");
60326154

60336155
goto no_values;
@@ -6042,11 +6164,11 @@ PHP_PGSQL_API int php_pgsql_insert(PGconn *pg_link, const char *table, zval *var
60426164
}
60436165
var_array = converted;
60446166
}
6045-
6167+
60466168
smart_str_appends(&querystr, "INSERT INTO ");
6047-
smart_str_appends(&querystr, table);
6169+
build_tablename(&querystr, pg_link, table);
60486170
smart_str_appends(&querystr, " (");
6049-
6171+
60506172
zend_hash_internal_pointer_reset_ex(Z_ARRVAL_P(var_array), &pos);
60516173
while ((key_type = zend_hash_get_current_key_ex(Z_ARRVAL_P(var_array), &fld,
60526174
&fld_len, &num_idx, 0, &pos)) != HASH_KEY_NON_EXISTANT) {
@@ -6233,7 +6355,7 @@ PHP_PGSQL_API int php_pgsql_update(PGconn *pg_link, const char *table, zval *var
62336355
}
62346356

62356357
smart_str_appends(&querystr, "UPDATE ");
6236-
smart_str_appends(&querystr, table);
6358+
build_tablename(&querystr, pg_link, table);
62376359
smart_str_appends(&querystr, " SET ");
62386360

62396361
if (build_assignment_string(&querystr, Z_ARRVAL_P(var_array), 0, ",", 1 TSRMLS_CC))
@@ -6334,7 +6456,7 @@ PHP_PGSQL_API int php_pgsql_delete(PGconn *pg_link, const char *table, zval *ids
63346456
}
63356457

63366458
smart_str_appends(&querystr, "DELETE FROM ");
6337-
smart_str_appends(&querystr, table);
6459+
build_tablename(&querystr, pg_link, table);
63386460
smart_str_appends(&querystr, " WHERE ");
63396461

63406462
if (build_assignment_string(&querystr, Z_ARRVAL_P(ids_array), 1, " AND ", sizeof(" AND ")-1 TSRMLS_CC))
@@ -6470,7 +6592,7 @@ PHP_PGSQL_API int php_pgsql_select(PGconn *pg_link, const char *table, zval *ids
64706592
}
64716593

64726594
smart_str_appends(&querystr, "SELECT * FROM ");
6473-
smart_str_appends(&querystr, table);
6595+
build_tablename(&querystr, pg_link, table);
64746596
smart_str_appends(&querystr, " WHERE ");
64756597

64766598
if (build_assignment_string(&querystr, Z_ARRVAL_P(ids_array), 1, " AND ", sizeof(" AND ")-1 TSRMLS_CC))

‎ext/pgsql/tests/10pg_convert.phpt

+6-6
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,10 @@ var_dump($converted);
2020
?>
2121
--EXPECT--
2222
array(3) {
23-
["num"]=>
23+
[""num""]=>
2424
string(4) "1234"
25-
["str"]=>
26-
string(5) "'AAA'"
27-
["bin"]=>
28-
string(5) "'BBB'"
29-
}
25+
[""str""]=>
26+
string(6) "E'AAA'"
27+
[""bin""]=>
28+
string(6) "E'BBB'"
29+
}

‎ext/pgsql/tests/10pg_convert_9.phpt

+6-6
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,10 @@ var_dump($converted);
2121
?>
2222
--EXPECT--
2323
array(3) {
24-
["num"]=>
24+
[""num""]=>
2525
string(4) "1234"
26-
["str"]=>
27-
string(5) "'AAA'"
28-
["bin"]=>
29-
string(11) "'\\x424242'"
30-
}
26+
[""str""]=>
27+
string(6) "E'AAA'"
28+
[""bin""]=>
29+
string(12) "E'\\x424242'"
30+
}

‎ext/pgsql/tests/12pg_insert.phpt

+2-2
Original file line numberDiff line numberDiff line change
@@ -20,5 +20,5 @@ echo pg_insert($db, $table_name, $fields, PGSQL_DML_STRING)."\n";
2020
echo "Ok\n";
2121
?>
2222
--EXPECT--
23-
INSERT INTO php_pgsql_test (num,str,bin) VALUES (1234,'AAA','BBB');
24-
Ok
23+
INSERT INTO "php_pgsql_test" ("num","str","bin") VALUES (1234,E'AAA',E'BBB');
24+
Ok

‎ext/pgsql/tests/12pg_insert_9.phpt

+2-2
Original file line numberDiff line numberDiff line change
@@ -22,5 +22,5 @@ echo pg_insert($db, $table_name, $fields, PGSQL_DML_STRING)."\n";
2222
echo "Ok\n";
2323
?>
2424
--EXPECT--
25-
INSERT INTO php_pgsql_test (num,str,bin) VALUES (1234,'AAA','\\x424242');
26-
Ok
25+
INSERT INTO "php_pgsql_test" ("num","str","bin") VALUES (1234,E'AAA',E'\\x424242');
26+
Ok

‎ext/pgsql/tests/13pg_select.phpt

+1-1
Original file line numberDiff line numberDiff line change
@@ -33,5 +33,5 @@ array(1) {
3333
string(3) "BBB"
3434
}
3535
}
36-
SELECT * FROM php_pgsql_test WHERE num=1234;
36+
SELECT * FROM "php_pgsql_test" WHERE "num"=1234;
3737
Ok

‎ext/pgsql/tests/13pg_select_9.phpt

+2-2
Original file line numberDiff line numberDiff line change
@@ -35,5 +35,5 @@ array(1) {
3535
string(8) "\x424242"
3636
}
3737
}
38-
SELECT * FROM php_pgsql_test WHERE num=1234;
39-
Ok
38+
SELECT * FROM "php_pgsql_test" WHERE "num"=1234;
39+
Ok

‎ext/pgsql/tests/14pg_update.phpt

+2-2
Original file line numberDiff line numberDiff line change
@@ -21,5 +21,5 @@ echo pg_update($db, $table_name, $fields, $ids, PGSQL_DML_STRING)."\n";
2121
echo "Ok\n";
2222
?>
2323
--EXPECT--
24-
UPDATE php_pgsql_test SET num=1234,str='ABC',bin='XYZ' WHERE num=1234;
25-
Ok
24+
UPDATE "php_pgsql_test" SET "num"=1234,"str"=E'ABC',"bin"=E'XYZ' WHERE "num"=1234;
25+
Ok

‎ext/pgsql/tests/14pg_update_9.phpt

+2-2
Original file line numberDiff line numberDiff line change
@@ -23,5 +23,5 @@ echo pg_update($db, $table_name, $fields, $ids, PGSQL_DML_STRING)."\n";
2323
echo "Ok\n";
2424
?>
2525
--EXPECT--
26-
UPDATE php_pgsql_test SET num=1234,str='ABC',bin='\\x58595a' WHERE num=1234;
27-
Ok
26+
UPDATE "php_pgsql_test" SET "num"=1234,"str"=E'ABC',"bin"=E'\\x58595a' WHERE "num"=1234;
27+
Ok

0 commit comments

Comments
 (0)