Skip to content

Commit 7ff2c22

Browse files
committed
ext/pgsql: escape table name, delimiter, null marker in pg_copy_from/to
The COPY query embedded the table_name argument with a raw "%s" and the delimiter and null marker inside literal E'..' wrappers, so caller-supplied strings could break out and run side queries. Route bare table names through build_tablename (the same helper pg_insert/update/select/delete have used since bug #62978), and pass the delimiter and null marker through PQescapeLiteral. pg_copy_to keeps the parenthesised (query) source form documented in bug 73498, but wraps it in an extra paren pair so a string like (SELECT 1); DROP TABLE x; -- becomes a syntax error inside the outer parens instead of escaping out into a second statement. Closes GH-21985
1 parent 8d0777e commit 7ff2c22

6 files changed

Lines changed: 286 additions & 12 deletions

ext/pgsql/pgsql.c

Lines changed: 61 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,8 @@ static void pgsql_lob_free_obj(zend_object *obj)
273273

274274
/* Compatibility definitions */
275275

276+
static inline zend_result build_tablename(smart_str *querystr, PGconn *pg_link, const zend_string *table);
277+
276278
static zend_string *_php_pgsql_trim_message(const char *message)
277279
{
278280
size_t i = strlen(message);
@@ -3347,9 +3349,8 @@ PHP_FUNCTION(pg_copy_to)
33473349
pgsql_link_handle *link;
33483350
zend_string *table_name;
33493351
zend_string *pg_delimiter = NULL;
3350-
char *pg_null_as = "\\\\N";
3351-
size_t pg_null_as_len = 0;
3352-
char *query;
3352+
char *pg_null_as = "\\N";
3353+
size_t pg_null_as_len = sizeof("\\N") - 1;
33533354
PGconn *pgsql;
33543355
PGresult *pgsql_result;
33553356
ExecStatusType status;
@@ -3373,14 +3374,40 @@ PHP_FUNCTION(pg_copy_to)
33733374
zend_argument_value_error(3, "must be one character");
33743375
RETURN_THROWS();
33753376
}
3377+
smart_str querystr = {0};
3378+
smart_str_appends(&querystr, "COPY ");
3379+
if (ZSTR_LEN(table_name) > 0 && ZSTR_VAL(table_name)[0] == '(') {
3380+
smart_str_appendc(&querystr, '(');
3381+
smart_str_append(&querystr, table_name);
3382+
smart_str_appendc(&querystr, ')');
3383+
} else if (build_tablename(&querystr, pgsql, table_name) == FAILURE) {
3384+
smart_str_free(&querystr);
3385+
RETURN_FALSE;
3386+
}
33763387

3377-
spprintf(&query, 0, "COPY %s TO STDOUT DELIMITER E'%c' NULL AS E'%s'", ZSTR_VAL(table_name), *ZSTR_VAL(pg_delimiter), pg_null_as);
3388+
char *escaped_delimiter = PQescapeLiteral(pgsql, ZSTR_VAL(pg_delimiter), 1);
3389+
if (!escaped_delimiter) {
3390+
php_error_docref(NULL, E_WARNING, "Failed to escape delimiter");
3391+
smart_str_free(&querystr);
3392+
RETURN_FALSE;
3393+
}
3394+
char *escaped_null_as = PQescapeLiteral(pgsql, pg_null_as, pg_null_as_len);
3395+
if (!escaped_null_as) {
3396+
php_error_docref(NULL, E_WARNING, "Failed to escape null_as");
3397+
PQfreemem(escaped_delimiter);
3398+
smart_str_free(&querystr);
3399+
RETURN_FALSE;
3400+
}
3401+
smart_str_append_printf(&querystr, " TO STDOUT DELIMITER %s NULL AS %s", escaped_delimiter, escaped_null_as);
3402+
smart_str_0(&querystr);
3403+
PQfreemem(escaped_delimiter);
3404+
PQfreemem(escaped_null_as);
33783405

33793406
while ((pgsql_result = PQgetResult(pgsql))) {
33803407
PQclear(pgsql_result);
33813408
}
3382-
pgsql_result = PQexec(pgsql, query);
3383-
efree(query);
3409+
pgsql_result = PQexec(pgsql, ZSTR_VAL(querystr.s));
3410+
smart_str_free(&querystr);
33843411

33853412
if (pgsql_result) {
33863413
status = PQresultStatus(pgsql_result);
@@ -3462,9 +3489,8 @@ PHP_FUNCTION(pg_copy_from)
34623489
zval *value;
34633490
zend_string *table_name;
34643491
zend_string *pg_delimiter = NULL;
3465-
char *pg_null_as = "\\\\N";
3466-
size_t pg_null_as_len;
3467-
char *query;
3492+
char *pg_null_as = "\\N";
3493+
size_t pg_null_as_len = sizeof("\\N") - 1;
34683494
PGconn *pgsql;
34693495
PGresult *pgsql_result;
34703496
ExecStatusType status;
@@ -3488,14 +3514,37 @@ PHP_FUNCTION(pg_copy_from)
34883514
zend_argument_value_error(4, "must be one character");
34893515
RETURN_THROWS();
34903516
}
3517+
smart_str querystr = {0};
3518+
smart_str_appends(&querystr, "COPY ");
3519+
if (build_tablename(&querystr, pgsql, table_name) == FAILURE) {
3520+
smart_str_free(&querystr);
3521+
RETURN_FALSE;
3522+
}
3523+
3524+
char *escaped_delimiter = PQescapeLiteral(pgsql, ZSTR_VAL(pg_delimiter), 1);
3525+
if (!escaped_delimiter) {
3526+
php_error_docref(NULL, E_WARNING, "Failed to escape delimiter");
3527+
smart_str_free(&querystr);
3528+
RETURN_FALSE;
3529+
}
3530+
char *escaped_null_as = PQescapeLiteral(pgsql, pg_null_as, pg_null_as_len);
3531+
if (!escaped_null_as) {
3532+
php_error_docref(NULL, E_WARNING, "Failed to escape null_as");
3533+
PQfreemem(escaped_delimiter);
3534+
smart_str_free(&querystr);
3535+
RETURN_FALSE;
3536+
}
3537+
smart_str_append_printf(&querystr, " FROM STDIN DELIMITER %s NULL AS %s", escaped_delimiter, escaped_null_as);
3538+
smart_str_0(&querystr);
3539+
PQfreemem(escaped_delimiter);
3540+
PQfreemem(escaped_null_as);
34913541

3492-
spprintf(&query, 0, "COPY %s FROM STDIN DELIMITER E'%c' NULL AS E'%s'", ZSTR_VAL(table_name), *ZSTR_VAL(pg_delimiter), pg_null_as);
34933542
while ((pgsql_result = PQgetResult(pgsql))) {
34943543
PQclear(pgsql_result);
34953544
}
3496-
pgsql_result = PQexec(pgsql, query);
3545+
pgsql_result = PQexec(pgsql, ZSTR_VAL(querystr.s));
34973546

3498-
efree(query);
3547+
smart_str_free(&querystr);
34993548

35003549
if (pgsql_result) {
35013550
status = PQresultStatus(pgsql_result);
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
--TEST--
2+
pg_copy_to() / pg_copy_from() default null marker round-trip
3+
--EXTENSIONS--
4+
pgsql
5+
--SKIPIF--
6+
<?php include("inc/skipif.inc"); ?>
7+
--FILE--
8+
<?php
9+
10+
include('inc/config.inc');
11+
12+
$db = pg_connect($conn_str);
13+
pg_query($db, 'DROP TABLE IF EXISTS pg_copy_default_null');
14+
pg_query($db, 'CREATE TABLE pg_copy_default_null (id int, v text)');
15+
pg_query($db, "INSERT INTO pg_copy_default_null VALUES (1, 'hello'), (2, NULL)");
16+
17+
$rows = pg_copy_to($db, 'pg_copy_default_null');
18+
var_dump($rows);
19+
20+
pg_query($db, 'DELETE FROM pg_copy_default_null');
21+
var_dump(pg_copy_from($db, 'pg_copy_default_null', $rows));
22+
var_dump(pg_fetch_all(pg_query($db, 'SELECT v FROM pg_copy_default_null ORDER BY id')));
23+
24+
?>
25+
--CLEAN--
26+
<?php
27+
include('inc/config.inc');
28+
$db = pg_connect($conn_str);
29+
pg_query($db, 'DROP TABLE IF EXISTS pg_copy_default_null');
30+
?>
31+
--EXPECT--
32+
array(2) {
33+
[0]=>
34+
string(8) "1 hello
35+
"
36+
[1]=>
37+
string(5) "2 \N
38+
"
39+
}
40+
bool(true)
41+
array(2) {
42+
[0]=>
43+
array(1) {
44+
["v"]=>
45+
string(5) "hello"
46+
}
47+
[1]=>
48+
array(1) {
49+
["v"]=>
50+
NULL
51+
}
52+
}
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
--TEST--
2+
pg_copy_from() escapes the null_as argument
3+
--EXTENSIONS--
4+
pgsql
5+
--SKIPIF--
6+
<?php include("inc/skipif.inc"); ?>
7+
--FILE--
8+
<?php
9+
10+
include('inc/config.inc');
11+
12+
$db = pg_connect($conn_str);
13+
pg_query($db, 'DROP TABLE IF EXISTS pg_copy_null_as_target');
14+
pg_query($db, 'DROP TABLE IF EXISTS pg_copy_null_as_injected');
15+
pg_query($db, 'CREATE TABLE pg_copy_null_as_target (v text)');
16+
17+
$evil = "X'; CREATE TABLE pg_copy_null_as_injected (v text); --";
18+
var_dump(pg_copy_from($db, 'pg_copy_null_as_target', ["row\n"], "\t", $evil));
19+
20+
$r = pg_query($db, "SELECT 1 FROM pg_tables WHERE tablename = 'pg_copy_null_as_injected'");
21+
var_dump(pg_num_rows($r));
22+
23+
$r = pg_query($db, 'SELECT v FROM pg_copy_null_as_target ORDER BY v');
24+
var_dump(pg_fetch_all($r));
25+
26+
?>
27+
--CLEAN--
28+
<?php
29+
include('inc/config.inc');
30+
$db = pg_connect($conn_str);
31+
pg_query($db, 'DROP TABLE IF EXISTS pg_copy_null_as_target');
32+
pg_query($db, 'DROP TABLE IF EXISTS pg_copy_null_as_injected');
33+
?>
34+
--EXPECT--
35+
bool(true)
36+
int(0)
37+
array(1) {
38+
[0]=>
39+
array(1) {
40+
["v"]=>
41+
string(3) "row"
42+
}
43+
}
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
--TEST--
2+
pg_copy_from() escapes the table name argument
3+
--EXTENSIONS--
4+
pgsql
5+
--SKIPIF--
6+
<?php include("inc/skipif.inc"); ?>
7+
--FILE--
8+
<?php
9+
10+
include('inc/config.inc');
11+
12+
$db = pg_connect($conn_str);
13+
pg_query($db, 'DROP TABLE IF EXISTS pg_copy_from_target');
14+
pg_query($db, 'CREATE TABLE pg_copy_from_target (v text)');
15+
pg_query($db, 'DROP TABLE IF EXISTS pg_copy_from_other');
16+
pg_query($db, 'CREATE TABLE pg_copy_from_other (v text)');
17+
18+
$evil = "pg_copy_from_other FROM STDIN --";
19+
var_dump(pg_copy_from($db, $evil, ["redirected\n"]));
20+
21+
$rows = pg_fetch_all(pg_query($db, 'SELECT v FROM pg_copy_from_other')) ?: [];
22+
var_dump($rows);
23+
24+
?>
25+
--CLEAN--
26+
<?php
27+
include('inc/config.inc');
28+
$db = pg_connect($conn_str);
29+
pg_query($db, 'DROP TABLE IF EXISTS pg_copy_from_target');
30+
pg_query($db, 'DROP TABLE IF EXISTS pg_copy_from_other');
31+
?>
32+
--EXPECTF--
33+
Warning: pg_copy_from(): Copy command failed: ERROR:%srelation "pg_copy_from_other FROM STDIN --" does not exist%ain %s on line %d
34+
bool(false)
35+
array(0) {
36+
}
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
--TEST--
2+
pg_copy_to() (query) source form: parens-wrap, already-parenthesised, and statement-injection rejection
3+
--EXTENSIONS--
4+
pgsql
5+
--SKIPIF--
6+
<?php include("inc/skipif.inc"); ?>
7+
--FILE--
8+
<?php
9+
10+
include('inc/config.inc');
11+
12+
$db = pg_connect($conn_str);
13+
pg_query($db, 'DROP TABLE IF EXISTS pg_copy_to_qsource');
14+
pg_query($db, 'DROP TABLE IF EXISTS pg_copy_to_qinjected');
15+
pg_query($db, 'CREATE TABLE pg_copy_to_qsource (v text)');
16+
pg_query($db, "INSERT INTO pg_copy_to_qsource VALUES ('a'), ('b')");
17+
18+
var_dump(pg_copy_to($db, '(SELECT v FROM pg_copy_to_qsource ORDER BY v)'));
19+
20+
var_dump(pg_copy_to($db, '((SELECT v FROM pg_copy_to_qsource ORDER BY v))'));
21+
22+
$evil = '(SELECT 1); DROP TABLE pg_copy_to_qsource; --';
23+
var_dump(pg_copy_to($db, $evil));
24+
25+
$r = pg_query($db, "SELECT 1 FROM pg_tables WHERE tablename = 'pg_copy_to_qsource'");
26+
var_dump(pg_num_rows($r));
27+
28+
$r = pg_query($db, "SELECT 1 FROM pg_tables WHERE tablename = 'pg_copy_to_qinjected'");
29+
var_dump(pg_num_rows($r));
30+
31+
?>
32+
--CLEAN--
33+
<?php
34+
include('inc/config.inc');
35+
$db = pg_connect($conn_str);
36+
pg_query($db, 'DROP TABLE IF EXISTS pg_copy_to_qsource');
37+
pg_query($db, 'DROP TABLE IF EXISTS pg_copy_to_qinjected');
38+
?>
39+
--EXPECTF--
40+
array(2) {
41+
[0]=>
42+
string(2) "a
43+
"
44+
[1]=>
45+
string(2) "b
46+
"
47+
}
48+
array(2) {
49+
[0]=>
50+
string(2) "a
51+
"
52+
[1]=>
53+
string(2) "b
54+
"
55+
}
56+
57+
Warning: pg_copy_to(): Copy command failed: ERROR:%ssyntax error at or near ";"%ain %s on line %d
58+
bool(false)
59+
int(1)
60+
int(0)
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
--TEST--
2+
pg_copy_to() escapes the table name argument
3+
--EXTENSIONS--
4+
pgsql
5+
--SKIPIF--
6+
<?php include("inc/skipif.inc"); ?>
7+
--FILE--
8+
<?php
9+
10+
include('inc/config.inc');
11+
12+
$db = pg_connect($conn_str);
13+
pg_query($db, 'DROP TABLE IF EXISTS pg_copy_to_source');
14+
pg_query($db, 'DROP TABLE IF EXISTS pg_copy_to_injected');
15+
pg_query($db, 'CREATE TABLE pg_copy_to_source (v text)');
16+
17+
$evil = "pg_copy_to_source; CREATE TABLE pg_copy_to_injected (v text); --";
18+
var_dump(pg_copy_to($db, $evil));
19+
20+
$r = pg_query($db, "SELECT 1 FROM pg_tables WHERE tablename = 'pg_copy_to_injected'");
21+
var_dump(pg_num_rows($r));
22+
23+
?>
24+
--CLEAN--
25+
<?php
26+
include('inc/config.inc');
27+
$db = pg_connect($conn_str);
28+
pg_query($db, 'DROP TABLE IF EXISTS pg_copy_to_source');
29+
pg_query($db, 'DROP TABLE IF EXISTS pg_copy_to_injected');
30+
?>
31+
--EXPECTF--
32+
Warning: pg_copy_to(): Copy command failed: ERROR:%srelation "%s" does not exist%ain %s on line %d
33+
bool(false)
34+
int(0)

0 commit comments

Comments
 (0)