-
Notifications
You must be signed in to change notification settings - Fork 8k
ext/pgsql: route pg_copy_from table_name through build_tablename #21985
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -273,6 +273,60 @@ static void pgsql_lob_free_obj(zend_object *obj) | |
|
|
||
| /* Compatibility definitions */ | ||
|
|
||
| static zend_result build_tablename(smart_str *querystr, PGconn *pg_link, const zend_string *table); | ||
|
|
||
| static bool pgsql_copy_table_name_is_simple(const char *s, size_t len) | ||
| { | ||
| if (len == 0) { | ||
| return false; | ||
| } | ||
| size_t i = 0; | ||
| if (!(isalpha((unsigned char) s[i]) || s[i] == '_')) { | ||
| return false; | ||
| } | ||
| i++; | ||
| while (i < len && (isalnum((unsigned char) s[i]) || s[i] == '_')) { | ||
| i++; | ||
| } | ||
| if (i == len) { | ||
| return true; | ||
| } | ||
| if (s[i] != '.') { | ||
| return false; | ||
| } | ||
| i++; | ||
| if (i >= len || !(isalpha((unsigned char) s[i]) || s[i] == '_')) { | ||
| return false; | ||
| } | ||
| i++; | ||
| while (i < len && (isalnum((unsigned char) s[i]) || s[i] == '_')) { | ||
| i++; | ||
| } | ||
| return i == len; | ||
| } | ||
|
|
||
| static bool pgsql_copy_query_form_balanced(const char *s, size_t len) | ||
| { | ||
| if (len < 2 || s[0] != '(' || s[len - 1] != ')') { | ||
| return false; | ||
| } | ||
| int depth = 0; | ||
| for (size_t i = 0; i < len; i++) { | ||
| if (s[i] == '(') { | ||
| depth++; | ||
| } else if (s[i] == ')') { | ||
| depth--; | ||
| if (depth < 0) { | ||
| return false; | ||
| } | ||
| if (depth == 0 && i != len - 1) { | ||
| return false; | ||
| } | ||
| } | ||
| } | ||
| return depth == 0; | ||
| } | ||
|
|
||
| static zend_string *_php_pgsql_trim_message(const char *message) | ||
| { | ||
| size_t i = strlen(message); | ||
|
|
@@ -3347,9 +3401,8 @@ PHP_FUNCTION(pg_copy_to) | |
| pgsql_link_handle *link; | ||
| zend_string *table_name; | ||
| zend_string *pg_delimiter = NULL; | ||
| char *pg_null_as = "\\\\N"; | ||
| size_t pg_null_as_len = 0; | ||
| char *query; | ||
| char *pg_null_as = "\\N"; | ||
| size_t pg_null_as_len = sizeof("\\N") - 1; | ||
| PGconn *pgsql; | ||
| PGresult *pgsql_result; | ||
| ExecStatusType status; | ||
|
|
@@ -3373,14 +3426,55 @@ PHP_FUNCTION(pg_copy_to) | |
| zend_argument_value_error(3, "must be one character"); | ||
| RETURN_THROWS(); | ||
| } | ||
| bool is_query_form = ZSTR_LEN(table_name) > 0 && ZSTR_VAL(table_name)[0] == '('; | ||
| if (is_query_form) { | ||
| if (!pgsql_copy_query_form_balanced(ZSTR_VAL(table_name), ZSTR_LEN(table_name))) { | ||
| php_error_docref(NULL, E_WARNING, "Invalid query source '%s': must be a single balanced parenthesised expression", ZSTR_VAL(table_name)); | ||
| RETURN_FALSE; | ||
| } | ||
| } else if (!pgsql_copy_table_name_is_simple(ZSTR_VAL(table_name), ZSTR_LEN(table_name))) { | ||
| php_error_docref(NULL, E_WARNING, "Invalid table_name '%s': must be a plain identifier or schema.table", ZSTR_VAL(table_name)); | ||
| RETURN_FALSE; | ||
| } | ||
|
|
||
| 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); | ||
| smart_str querystr = {0}; | ||
| smart_str_appends(&querystr, "COPY "); | ||
| if (is_query_form) { | ||
| smart_str_appendc(&querystr, '('); | ||
| smart_str_append(&querystr, table_name); | ||
| smart_str_appendc(&querystr, ')'); | ||
| } else if (build_tablename(&querystr, pgsql, table_name) == FAILURE) { | ||
| smart_str_free(&querystr); | ||
| RETURN_FALSE; | ||
| } | ||
|
|
||
| char *escaped_delimiter = PQescapeLiteral(pgsql, ZSTR_VAL(pg_delimiter), 1); | ||
| if (!escaped_delimiter) { | ||
| zend_string *msgbuf = _php_pgsql_trim_message(PQerrorMessage(pgsql)); | ||
| php_error_docref(NULL, E_WARNING, "Failed to escape delimiter '%c': %s", *ZSTR_VAL(pg_delimiter), ZSTR_VAL(msgbuf)); | ||
| zend_string_release(msgbuf); | ||
| smart_str_free(&querystr); | ||
| RETURN_FALSE; | ||
| } | ||
| char *escaped_null_as = PQescapeLiteral(pgsql, pg_null_as, pg_null_as_len); | ||
| if (!escaped_null_as) { | ||
| zend_string *msgbuf = _php_pgsql_trim_message(PQerrorMessage(pgsql)); | ||
| php_error_docref(NULL, E_WARNING, "Failed to escape null_as '%s': %s", pg_null_as, ZSTR_VAL(msgbuf)); | ||
| zend_string_release(msgbuf); | ||
| PQfreemem(escaped_delimiter); | ||
| smart_str_free(&querystr); | ||
| RETURN_FALSE; | ||
| } | ||
| smart_str_append_printf(&querystr, " TO STDOUT DELIMITER %s NULL AS %s", escaped_delimiter, escaped_null_as); | ||
| smart_str_0(&querystr); | ||
| PQfreemem(escaped_delimiter); | ||
| PQfreemem(escaped_null_as); | ||
|
|
||
| while ((pgsql_result = PQgetResult(pgsql))) { | ||
| PQclear(pgsql_result); | ||
| } | ||
| pgsql_result = PQexec(pgsql, query); | ||
| efree(query); | ||
| pgsql_result = PQexec(pgsql, ZSTR_VAL(querystr.s)); | ||
| smart_str_free(&querystr); | ||
|
|
||
| if (pgsql_result) { | ||
| status = PQresultStatus(pgsql_result); | ||
|
|
@@ -3462,9 +3556,8 @@ PHP_FUNCTION(pg_copy_from) | |
| zval *value; | ||
| zend_string *table_name; | ||
| zend_string *pg_delimiter = NULL; | ||
| char *pg_null_as = "\\\\N"; | ||
| size_t pg_null_as_len; | ||
| char *query; | ||
| char *pg_null_as = "\\N"; | ||
| size_t pg_null_as_len = sizeof("\\N") - 1; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. thinking of it, that looks a potential bug fix on its own (uninitialised optional param).
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suppose, when PR is deemed ready (one day soon I hope 😆), the question would be if it back-port or not and decision is not to backport, this bit could stand on its own for 8.4/8.5 versions.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you can always open a PR then you ll rebase this one on it eventually. |
||
| PGconn *pgsql; | ||
| PGresult *pgsql_result; | ||
| ExecStatusType status; | ||
|
|
@@ -3488,14 +3581,46 @@ PHP_FUNCTION(pg_copy_from) | |
| zend_argument_value_error(4, "must be one character"); | ||
| RETURN_THROWS(); | ||
| } | ||
| if (!pgsql_copy_table_name_is_simple(ZSTR_VAL(table_name), ZSTR_LEN(table_name))) { | ||
| php_error_docref(NULL, E_WARNING, "Invalid table_name '%s': must be a plain identifier or schema.table", ZSTR_VAL(table_name)); | ||
| RETURN_FALSE; | ||
| } | ||
|
|
||
| smart_str querystr = {0}; | ||
| smart_str_appends(&querystr, "COPY "); | ||
| if (build_tablename(&querystr, pgsql, table_name) == FAILURE) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ditto
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same gate applied here. |
||
| smart_str_free(&querystr); | ||
| RETURN_FALSE; | ||
| } | ||
|
|
||
| char *escaped_delimiter = PQescapeLiteral(pgsql, ZSTR_VAL(pg_delimiter), 1); | ||
| if (!escaped_delimiter) { | ||
| zend_string *msgbuf = _php_pgsql_trim_message(PQerrorMessage(pgsql)); | ||
| php_error_docref(NULL, E_WARNING, "Failed to escape delimiter '%c': %s", *ZSTR_VAL(pg_delimiter), ZSTR_VAL(msgbuf)); | ||
| zend_string_release(msgbuf); | ||
| smart_str_free(&querystr); | ||
| RETURN_FALSE; | ||
| } | ||
| char *escaped_null_as = PQescapeLiteral(pgsql, pg_null_as, pg_null_as_len); | ||
| if (!escaped_null_as) { | ||
| zend_string *msgbuf = _php_pgsql_trim_message(PQerrorMessage(pgsql)); | ||
| php_error_docref(NULL, E_WARNING, "Failed to escape null_as '%s': %s", pg_null_as, ZSTR_VAL(msgbuf)); | ||
| zend_string_release(msgbuf); | ||
| PQfreemem(escaped_delimiter); | ||
| smart_str_free(&querystr); | ||
| RETURN_FALSE; | ||
| } | ||
| smart_str_append_printf(&querystr, " FROM STDIN DELIMITER %s NULL AS %s", escaped_delimiter, escaped_null_as); | ||
| smart_str_0(&querystr); | ||
| PQfreemem(escaped_delimiter); | ||
| PQfreemem(escaped_null_as); | ||
|
|
||
| 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); | ||
| while ((pgsql_result = PQgetResult(pgsql))) { | ||
| PQclear(pgsql_result); | ||
| } | ||
| pgsql_result = PQexec(pgsql, query); | ||
| pgsql_result = PQexec(pgsql, ZSTR_VAL(querystr.s)); | ||
|
|
||
| efree(query); | ||
| smart_str_free(&querystr); | ||
|
|
||
| if (pgsql_result) { | ||
| status = PQresultStatus(pgsql_result); | ||
|
|
@@ -5563,7 +5688,7 @@ static bool do_exec(smart_str *querystr, ExecStatusType expect, PGconn *pg_link, | |
| } | ||
| /* }}} */ | ||
|
|
||
| static inline zend_result build_tablename(smart_str *querystr, PGconn *pg_link, const zend_string *table) /* {{{ */ | ||
| static zend_result build_tablename(smart_str *querystr, PGconn *pg_link, const zend_string *table) /* {{{ */ | ||
| { | ||
| /* schema.table should be "schema"."table" */ | ||
| const char *dot = memchr(ZSTR_VAL(table), '.', ZSTR_LEN(table)); | ||
|
|
@@ -5574,7 +5699,9 @@ static inline zend_result build_tablename(smart_str *querystr, PGconn *pg_link, | |
| } else { | ||
| char *escaped = PQescapeIdentifier(pg_link, ZSTR_VAL(table), len); | ||
| if (escaped == NULL) { | ||
| php_error_docref(NULL, E_NOTICE, "Failed to escape table name '%s'", ZSTR_VAL(table)); | ||
| zend_string *msgbuf = _php_pgsql_trim_message(PQerrorMessage(pg_link)); | ||
| php_error_docref(NULL, E_WARNING, "Failed to escape table name '%s': %s", ZSTR_VAL(table), ZSTR_VAL(msgbuf)); | ||
| zend_string_release(msgbuf); | ||
| return FAILURE; | ||
| } | ||
| smart_str_appends(querystr, escaped); | ||
|
|
@@ -5590,7 +5717,9 @@ static inline zend_result build_tablename(smart_str *querystr, PGconn *pg_link, | |
| } else { | ||
| char *escaped = PQescapeIdentifier(pg_link, after_dot, len); | ||
| if (escaped == NULL) { | ||
| php_error_docref(NULL, E_NOTICE, "Failed to escape table name '%s'", ZSTR_VAL(table)); | ||
| zend_string *msgbuf = _php_pgsql_trim_message(PQerrorMessage(pg_link)); | ||
| php_error_docref(NULL, E_WARNING, "Failed to escape table name '%s': %s", ZSTR_VAL(table), ZSTR_VAL(msgbuf)); | ||
| zend_string_release(msgbuf); | ||
| return FAILURE; | ||
| } | ||
| smart_str_appendc(querystr, '.'); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| --TEST-- | ||
| pg_copy_to() / pg_copy_from() default null marker round-trip | ||
| --EXTENSIONS-- | ||
| pgsql | ||
| --SKIPIF-- | ||
| <?php include("inc/skipif.inc"); ?> | ||
| --FILE-- | ||
| <?php | ||
|
|
||
| include('inc/config.inc'); | ||
|
|
||
| $db = pg_connect($conn_str); | ||
| pg_query($db, 'DROP TABLE IF EXISTS pg_copy_default_null'); | ||
| pg_query($db, 'CREATE TABLE pg_copy_default_null (id int, v text)'); | ||
| pg_query($db, "INSERT INTO pg_copy_default_null VALUES (1, 'hello'), (2, NULL)"); | ||
|
|
||
| $rows = pg_copy_to($db, 'pg_copy_default_null'); | ||
| var_dump($rows); | ||
|
|
||
| pg_query($db, 'DELETE FROM pg_copy_default_null'); | ||
| var_dump(pg_copy_from($db, 'pg_copy_default_null', $rows)); | ||
| var_dump(pg_fetch_all(pg_query($db, 'SELECT v FROM pg_copy_default_null ORDER BY id'))); | ||
|
|
||
| ?> | ||
| --CLEAN-- | ||
| <?php | ||
| include('inc/config.inc'); | ||
| $db = pg_connect($conn_str); | ||
| pg_query($db, 'DROP TABLE IF EXISTS pg_copy_default_null'); | ||
| ?> | ||
| --EXPECT-- | ||
| array(2) { | ||
| [0]=> | ||
| string(8) "1 hello | ||
| " | ||
| [1]=> | ||
| string(5) "2 \N | ||
| " | ||
| } | ||
| bool(true) | ||
| array(2) { | ||
| [0]=> | ||
| array(1) { | ||
| ["v"]=> | ||
| string(5) "hello" | ||
| } | ||
| [1]=> | ||
| array(1) { | ||
| ["v"]=> | ||
| NULL | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| --TEST-- | ||
| pg_copy_from() escapes the null_as argument | ||
| --EXTENSIONS-- | ||
| pgsql | ||
| --SKIPIF-- | ||
| <?php include("inc/skipif.inc"); ?> | ||
| --FILE-- | ||
| <?php | ||
|
|
||
| include('inc/config.inc'); | ||
|
|
||
| $db = pg_connect($conn_str); | ||
| pg_query($db, 'DROP TABLE IF EXISTS pg_copy_null_as_target'); | ||
| pg_query($db, 'DROP TABLE IF EXISTS pg_copy_null_as_injected'); | ||
| pg_query($db, 'CREATE TABLE pg_copy_null_as_target (v text)'); | ||
|
|
||
| $evil = "X'; CREATE TABLE pg_copy_null_as_injected (v text); --"; | ||
| var_dump(pg_copy_from($db, 'pg_copy_null_as_target', ["row\n"], "\t", $evil)); | ||
|
|
||
| $r = pg_query($db, "SELECT 1 FROM pg_tables WHERE tablename = 'pg_copy_null_as_injected'"); | ||
| var_dump(pg_num_rows($r)); | ||
|
|
||
| $r = pg_query($db, 'SELECT v FROM pg_copy_null_as_target ORDER BY v'); | ||
| var_dump(pg_fetch_all($r)); | ||
|
|
||
| ?> | ||
| --CLEAN-- | ||
| <?php | ||
| include('inc/config.inc'); | ||
| $db = pg_connect($conn_str); | ||
| pg_query($db, 'DROP TABLE IF EXISTS pg_copy_null_as_target'); | ||
| pg_query($db, 'DROP TABLE IF EXISTS pg_copy_null_as_injected'); | ||
| ?> | ||
| --EXPECT-- | ||
| bool(true) | ||
| int(0) | ||
| array(1) { | ||
| [0]=> | ||
| array(1) { | ||
| ["v"]=> | ||
| string(3) "row" | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| --TEST-- | ||
| pg_copy_from() escapes the table name argument | ||
| --EXTENSIONS-- | ||
| pgsql | ||
| --SKIPIF-- | ||
| <?php include("inc/skipif.inc"); ?> | ||
| --FILE-- | ||
| <?php | ||
|
|
||
| include('inc/config.inc'); | ||
|
|
||
| $db = pg_connect($conn_str); | ||
| pg_query($db, 'DROP TABLE IF EXISTS pg_copy_from_target'); | ||
| pg_query($db, 'CREATE TABLE pg_copy_from_target (v text)'); | ||
| pg_query($db, 'DROP TABLE IF EXISTS pg_copy_from_other'); | ||
| pg_query($db, 'CREATE TABLE pg_copy_from_other (v text)'); | ||
|
|
||
| $evil = "pg_copy_from_other FROM STDIN --"; | ||
| var_dump(pg_copy_from($db, $evil, ["redirected\n"])); | ||
|
|
||
| $rows = pg_fetch_all(pg_query($db, 'SELECT v FROM pg_copy_from_other')) ?: []; | ||
| var_dump($rows); | ||
|
|
||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you add a case that tries to close the wrapper early, like '(SELECT 1)); DROP TABLE pg_copy_to_qsource; --', plus the pg_tables check?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added the wrapper-close test case plus a paren-balance check on the (query) input: must start with |
||
| ?> | ||
| --CLEAN-- | ||
| <?php | ||
| include('inc/config.inc'); | ||
| $db = pg_connect($conn_str); | ||
| pg_query($db, 'DROP TABLE IF EXISTS pg_copy_from_target'); | ||
| pg_query($db, 'DROP TABLE IF EXISTS pg_copy_from_other'); | ||
| ?> | ||
| --EXPECTF-- | ||
| Warning: pg_copy_from(): Invalid table_name 'pg_copy_from_other FROM STDIN --': must be a plain identifier or schema.table in %s on line %d | ||
| bool(false) | ||
| array(0) { | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens with such case ?
(SELECT ')' FROM t)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be false negative and be rejected, same as
(SELECT '(' FROM t).I could probably add single quote support without too much trouble, but that's not the only escape sequence in pgsql like $$ and double quotes for identifiers which would be more complex. Not sure how far we want to take this