agora inbox for pgsql-committers@postgresql.orghelp / color / mirror / Atom feed
pgsql: Teach psql to skip in-line COPY ... FROM STDIN data after a fail 5+ messages / 1 participants [nested] [flat]
* pgsql: Teach psql to skip in-line COPY ... FROM STDIN data after a fail @ 2026-08-10 13:41 Noah Misch <noah@leadboat.com> 0 siblings, 0 replies; 5+ messages in thread From: Noah Misch @ 2026-08-10 13:41 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Teach psql to skip in-line COPY ... FROM STDIN data after a failure. If the COPY command fails before sending PGRES_COPY_IN, psql did not realize that it ought to consume any in-line data following the command. Failing to do so leads to trying to execute that data as SQL commands, which in the best case is wrong and in the worst case is a SQL-injection hazard. To fix: 1. Extend psqlscan.l to recognize COPY ... FROM STDIN. This can be done with a pretty simple extension to the logic that already recognizes nested BEGIN blocks within CREATE FUNCTION et al. But unlike that case, we need to consider and count multiple COPY commands within a single query string (separated by "\;"). The fallout from that is that psql_scan_reset must now always be called before starting a new query string. (The comment for it that claimed we didn't need that because "the scan state must be INITIAL" was really obsolete already, since it has long reset more state besides start_state.) 2. Teach handleCopyIn() to read and discard data when passed NULL for "conn". 3. Add logic to SendQuery() to call handleCopyIn() that way if the query string contained COPY ... FROM STDIN command(s) that remain unaccounted-for at the end. Now that we have this counting logic, we can also detect if the backend sends an unexpected PGRES_COPY_IN message. That should never happen, but perhaps a malicious server could try to extract data that way. A side-effect of doing this is that we have to adjust a number of test scripts that thought they needn't write "\." after a COPY FROM STDIN that they expect to fail. On the whole this is an improvement, since there's now a uniform rule "write \. after COPY FROM STDIN, whether you expect it to work or not". But it is an annoying amount of test churn. A loose end in this patch is that if it has to skip data, it assumes that that data is text not binary. It seems unduly difficult to detect whether the COPY command requested binary (we could handle the old-style COPY BINARY ... syntax, but not the new style with format options). In practice, copying in-line binary data is unsupported anyway, because there's no way to write an end marker: the textual terminator sequence "\n\\.\n" could appear in binary data and there's no provision for escaping it, so neither psql nor the server look for it when in binary mode. Reported-by: Alexander Lakhin <exclusion@gmail.com> Author: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Noah Misch <noah@leadboat.com> Backpatch-through: 14 Security: CVE-2026-6464 Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/3045a25ba813b554e23c723ebe94cf504feb370c Author: Tom Lane <tgl@sss.pgh.pa.us> Modified Files -------------- src/bin/psql/common.c | 109 +++++++++++++++++++++++++---- src/bin/psql/common.h | 2 +- src/bin/psql/copy.c | 48 +++++++------ src/bin/psql/mainloop.c | 15 ++-- src/bin/psql/psqlscanslash.l | 2 + src/bin/psql/startup.c | 2 +- src/fe_utils/psqlscan.l | 88 +++++++++++++++++++++-- src/include/fe_utils/psqlscan.h | 2 + src/include/fe_utils/psqlscan_int.h | 11 +-- src/test/regress/expected/copy.out | 4 -- src/test/regress/expected/psql.out | 5 ++ src/test/regress/sql/alter_table.sql | 2 + src/test/regress/sql/copy.sql | 2 + src/test/regress/sql/copy2.sql | 53 ++++++++++++++ src/test/regress/sql/copyselect.sql | 1 + src/test/regress/sql/generated_stored.sql | 4 ++ src/test/regress/sql/generated_virtual.sql | 4 ++ src/test/regress/sql/privileges.sql | 3 + src/test/regress/sql/psql.sql | 9 +++ src/test/regress/sql/rowsecurity.sql | 4 ++ 20 files changed, 313 insertions(+), 57 deletions(-) ^ permalink raw reply [nested|flat] 5+ messages in thread
* pgsql: Teach psql to skip in-line COPY ... FROM STDIN data after a fail @ 2026-08-10 13:41 Noah Misch <noah@leadboat.com> 0 siblings, 0 replies; 5+ messages in thread From: Noah Misch @ 2026-08-10 13:41 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Teach psql to skip in-line COPY ... FROM STDIN data after a failure. If the COPY command fails before sending PGRES_COPY_IN, psql did not realize that it ought to consume any in-line data following the command. Failing to do so leads to trying to execute that data as SQL commands, which in the best case is wrong and in the worst case is a SQL-injection hazard. To fix: 1. Extend psqlscan.l to recognize COPY ... FROM STDIN. This can be done with a pretty simple extension to the logic that already recognizes nested BEGIN blocks within CREATE FUNCTION et al. But unlike that case, we need to consider and count multiple COPY commands within a single query string (separated by "\;"). The fallout from that is that psql_scan_reset must now always be called before starting a new query string. (The comment for it that claimed we didn't need that because "the scan state must be INITIAL" was really obsolete already, since it has long reset more state besides start_state.) 2. Teach handleCopyIn() to read and discard data when passed NULL for "conn". 3. Add logic to SendQuery() to call handleCopyIn() that way if the query string contained COPY ... FROM STDIN command(s) that remain unaccounted-for at the end. Now that we have this counting logic, we can also detect if the backend sends an unexpected PGRES_COPY_IN message. That should never happen, but perhaps a malicious server could try to extract data that way. A side-effect of doing this is that we have to adjust a number of test scripts that thought they needn't write "\." after a COPY FROM STDIN that they expect to fail. On the whole this is an improvement, since there's now a uniform rule "write \. after COPY FROM STDIN, whether you expect it to work or not". But it is an annoying amount of test churn. A loose end in this patch is that if it has to skip data, it assumes that that data is text not binary. It seems unduly difficult to detect whether the COPY command requested binary (we could handle the old-style COPY BINARY ... syntax, but not the new style with format options). In practice, copying in-line binary data is unsupported anyway, because there's no way to write an end marker: the textual terminator sequence "\n\\.\n" could appear in binary data and there's no provision for escaping it, so neither psql nor the server look for it when in binary mode. Reported-by: Alexander Lakhin <exclusion@gmail.com> Author: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Noah Misch <noah@leadboat.com> Backpatch-through: 14 Security: CVE-2026-6464 Branch ------ REL_19_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/d6ab88d374ab6bd6fead0902f7e7ff6ebc8b9006 Author: Tom Lane <tgl@sss.pgh.pa.us> Modified Files -------------- src/bin/psql/common.c | 109 +++++++++++++++++++++++++---- src/bin/psql/common.h | 2 +- src/bin/psql/copy.c | 48 +++++++------ src/bin/psql/mainloop.c | 15 ++-- src/bin/psql/psqlscanslash.l | 2 + src/bin/psql/startup.c | 2 +- src/fe_utils/psqlscan.l | 88 +++++++++++++++++++++-- src/include/fe_utils/psqlscan.h | 2 + src/include/fe_utils/psqlscan_int.h | 11 +-- src/test/regress/expected/copy.out | 4 -- src/test/regress/expected/psql.out | 5 ++ src/test/regress/sql/alter_table.sql | 2 + src/test/regress/sql/copy.sql | 2 + src/test/regress/sql/copy2.sql | 53 ++++++++++++++ src/test/regress/sql/copyselect.sql | 1 + src/test/regress/sql/generated_stored.sql | 4 ++ src/test/regress/sql/generated_virtual.sql | 4 ++ src/test/regress/sql/privileges.sql | 3 + src/test/regress/sql/psql.sql | 9 +++ src/test/regress/sql/rowsecurity.sql | 4 ++ 20 files changed, 313 insertions(+), 57 deletions(-) ^ permalink raw reply [nested|flat] 5+ messages in thread
* pgsql: Teach psql to skip in-line COPY ... FROM STDIN data after a fail @ 2026-08-10 13:41 Noah Misch <noah@leadboat.com> 0 siblings, 0 replies; 5+ messages in thread From: Noah Misch @ 2026-08-10 13:41 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Teach psql to skip in-line COPY ... FROM STDIN data after a failure. If the COPY command fails before sending PGRES_COPY_IN, psql did not realize that it ought to consume any in-line data following the command. Failing to do so leads to trying to execute that data as SQL commands, which in the best case is wrong and in the worst case is a SQL-injection hazard. To fix: 1. Extend psqlscan.l to recognize COPY ... FROM STDIN. This can be done with a pretty simple extension to the logic that already recognizes nested BEGIN blocks within CREATE FUNCTION et al. But unlike that case, we need to consider and count multiple COPY commands within a single query string (separated by "\;"). The fallout from that is that psql_scan_reset must now always be called before starting a new query string. (The comment for it that claimed we didn't need that because "the scan state must be INITIAL" was really obsolete already, since it has long reset more state besides start_state.) 2. Teach handleCopyIn() to read and discard data when passed NULL for "conn". 3. Add logic to SendQuery() to call handleCopyIn() that way if the query string contained COPY ... FROM STDIN command(s) that remain unaccounted-for at the end. Now that we have this counting logic, we can also detect if the backend sends an unexpected PGRES_COPY_IN message. That should never happen, but perhaps a malicious server could try to extract data that way. A side-effect of doing this is that we have to adjust a number of test scripts that thought they needn't write "\." after a COPY FROM STDIN that they expect to fail. On the whole this is an improvement, since there's now a uniform rule "write \. after COPY FROM STDIN, whether you expect it to work or not". But it is an annoying amount of test churn. A loose end in this patch is that if it has to skip data, it assumes that that data is text not binary. It seems unduly difficult to detect whether the COPY command requested binary (we could handle the old-style COPY BINARY ... syntax, but not the new style with format options). In practice, copying in-line binary data is unsupported anyway, because there's no way to write an end marker: the textual terminator sequence "\n\\.\n" could appear in binary data and there's no provision for escaping it, so neither psql nor the server look for it when in binary mode. Reported-by: Alexander Lakhin <exclusion@gmail.com> Author: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Noah Misch <noah@leadboat.com> Backpatch-through: 14 Security: CVE-2026-6464 Branch ------ REL_18_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/29921259e83b613a14d71d8b26fd5d0419944297 Author: Tom Lane <tgl@sss.pgh.pa.us> Modified Files -------------- src/bin/psql/common.c | 109 +++++++++++-- src/bin/psql/common.h | 2 +- src/bin/psql/copy.c | 48 +++--- src/bin/psql/mainloop.c | 15 +- src/bin/psql/psqlscanslash.l | 18 ++- src/bin/psql/startup.c | 2 +- src/fe_utils/psqlscan.l | 243 ++++++++++++++++++++++------- src/include/fe_utils/psqlscan.h | 2 + src/include/fe_utils/psqlscan_int.h | 15 +- src/test/regress/expected/copy.out | 4 - src/test/regress/expected/psql.out | 5 + src/test/regress/sql/alter_table.sql | 2 + src/test/regress/sql/copy.sql | 1 + src/test/regress/sql/copy2.sql | 46 ++++++ src/test/regress/sql/copyselect.sql | 1 + src/test/regress/sql/generated_stored.sql | 4 + src/test/regress/sql/generated_virtual.sql | 4 + src/test/regress/sql/privileges.sql | 3 + src/test/regress/sql/psql.sql | 9 ++ src/test/regress/sql/rowsecurity.sql | 4 + 20 files changed, 419 insertions(+), 118 deletions(-) ^ permalink raw reply [nested|flat] 5+ messages in thread
* pgsql: Teach psql to skip in-line COPY ... FROM STDIN data after a fail @ 2026-08-10 13:41 Noah Misch <noah@leadboat.com> 0 siblings, 0 replies; 5+ messages in thread From: Noah Misch @ 2026-08-10 13:41 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Teach psql to skip in-line COPY ... FROM STDIN data after a failure. If the COPY command fails before sending PGRES_COPY_IN, psql did not realize that it ought to consume any in-line data following the command. Failing to do so leads to trying to execute that data as SQL commands, which in the best case is wrong and in the worst case is a SQL-injection hazard. To fix: 1. Extend psqlscan.l to recognize COPY ... FROM STDIN. This can be done with a pretty simple extension to the logic that already recognizes nested BEGIN blocks within CREATE FUNCTION et al. But unlike that case, we need to consider and count multiple COPY commands within a single query string (separated by "\;"). The fallout from that is that psql_scan_reset must now always be called before starting a new query string. (The comment for it that claimed we didn't need that because "the scan state must be INITIAL" was really obsolete already, since it has long reset more state besides start_state.) 2. Teach handleCopyIn() to read and discard data when passed NULL for "conn". 3. Add logic to SendQuery() to call handleCopyIn() that way if the query string contained COPY ... FROM STDIN command(s) that remain unaccounted-for at the end. Now that we have this counting logic, we can also detect if the backend sends an unexpected PGRES_COPY_IN message. That should never happen, but perhaps a malicious server could try to extract data that way. A side-effect of doing this is that we have to adjust a number of test scripts that thought they needn't write "\." after a COPY FROM STDIN that they expect to fail. On the whole this is an improvement, since there's now a uniform rule "write \. after COPY FROM STDIN, whether you expect it to work or not". But it is an annoying amount of test churn. A loose end in this patch is that if it has to skip data, it assumes that that data is text not binary. It seems unduly difficult to detect whether the COPY command requested binary (we could handle the old-style COPY BINARY ... syntax, but not the new style with format options). In practice, copying in-line binary data is unsupported anyway, because there's no way to write an end marker: the textual terminator sequence "\n\\.\n" could appear in binary data and there's no provision for escaping it, so neither psql nor the server look for it when in binary mode. Reported-by: Alexander Lakhin <exclusion@gmail.com> Author: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Noah Misch <noah@leadboat.com> Backpatch-through: 14 Security: CVE-2026-6464 Branch ------ REL_17_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/46fa1f6f373865fae7553043790cd17944893ac3 Author: Tom Lane <tgl@sss.pgh.pa.us> Modified Files -------------- src/bin/psql/common.c | 109 ++++++++++++++-- src/bin/psql/common.h | 2 +- src/bin/psql/copy.c | 48 ++++--- src/bin/psql/mainloop.c | 15 ++- src/bin/psql/psqlscanslash.l | 18 +-- src/bin/psql/startup.c | 2 +- src/fe_utils/psqlscan.l | 243 ++++++++++++++++++++++++++--------- src/include/fe_utils/psqlscan.h | 2 + src/include/fe_utils/psqlscan_int.h | 15 ++- src/test/regress/expected/copy.out | 4 - src/test/regress/expected/psql.out | 5 + src/test/regress/sql/alter_table.sql | 2 + src/test/regress/sql/copy.sql | 1 + src/test/regress/sql/copy2.sql | 44 +++++++ src/test/regress/sql/copyselect.sql | 1 + src/test/regress/sql/generated.sql | 4 + src/test/regress/sql/privileges.sql | 3 + src/test/regress/sql/psql.sql | 9 ++ src/test/regress/sql/rowsecurity.sql | 4 + 19 files changed, 413 insertions(+), 118 deletions(-) ^ permalink raw reply [nested|flat] 5+ messages in thread
* pgsql: Teach psql to skip in-line COPY ... FROM STDIN data after a fail @ 2026-08-10 13:41 Noah Misch <noah@leadboat.com> 0 siblings, 0 replies; 5+ messages in thread From: Noah Misch @ 2026-08-10 13:41 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Teach psql to skip in-line COPY ... FROM STDIN data after a failure. If the COPY command fails before sending PGRES_COPY_IN, psql did not realize that it ought to consume any in-line data following the command. Failing to do so leads to trying to execute that data as SQL commands, which in the best case is wrong and in the worst case is a SQL-injection hazard. To fix: 1. Extend psqlscan.l to recognize COPY ... FROM STDIN. This can be done with a pretty simple extension to the logic that already recognizes nested BEGIN blocks within CREATE FUNCTION et al. But unlike that case, we need to consider and count multiple COPY commands within a single query string (separated by "\;"). The fallout from that is that psql_scan_reset must now always be called before starting a new query string. (The comment for it that claimed we didn't need that because "the scan state must be INITIAL" was really obsolete already, since it has long reset more state besides start_state.) 2. Teach handleCopyIn() to read and discard data when passed NULL for "conn". 3. Add logic to SendQuery() to call handleCopyIn() that way if the query string contained COPY ... FROM STDIN command(s) that remain unaccounted-for at the end. Now that we have this counting logic, we can also detect if the backend sends an unexpected PGRES_COPY_IN message. That should never happen, but perhaps a malicious server could try to extract data that way. A side-effect of doing this is that we have to adjust a number of test scripts that thought they needn't write "\." after a COPY FROM STDIN that they expect to fail. On the whole this is an improvement, since there's now a uniform rule "write \. after COPY FROM STDIN, whether you expect it to work or not". But it is an annoying amount of test churn. A loose end in this patch is that if it has to skip data, it assumes that that data is text not binary. It seems unduly difficult to detect whether the COPY command requested binary (we could handle the old-style COPY BINARY ... syntax, but not the new style with format options). In practice, copying in-line binary data is unsupported anyway, because there's no way to write an end marker: the textual terminator sequence "\n\\.\n" could appear in binary data and there's no provision for escaping it, so neither psql nor the server look for it when in binary mode. Reported-by: Alexander Lakhin <exclusion@gmail.com> Author: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Noah Misch <noah@leadboat.com> Backpatch-through: 14 Security: CVE-2026-6464 Branch ------ REL_16_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/2bdfd5cdcc09a04d281da6b731715016417f466e Author: Tom Lane <tgl@sss.pgh.pa.us> Modified Files -------------- src/bin/psql/common.c | 109 ++++++++++++++-- src/bin/psql/common.h | 2 +- src/bin/psql/copy.c | 48 ++++--- src/bin/psql/mainloop.c | 15 ++- src/bin/psql/psqlscanslash.l | 18 +-- src/bin/psql/startup.c | 2 +- src/fe_utils/psqlscan.l | 243 ++++++++++++++++++++++++++--------- src/include/fe_utils/psqlscan.h | 2 + src/include/fe_utils/psqlscan_int.h | 15 ++- src/test/regress/expected/copy.out | 4 - src/test/regress/expected/psql.out | 5 + src/test/regress/sql/alter_table.sql | 2 + src/test/regress/sql/copy.sql | 1 + src/test/regress/sql/copy2.sql | 33 +++++ src/test/regress/sql/copyselect.sql | 1 + src/test/regress/sql/generated.sql | 4 + src/test/regress/sql/privileges.sql | 3 + src/test/regress/sql/psql.sql | 9 ++ src/test/regress/sql/rowsecurity.sql | 4 + 19 files changed, 402 insertions(+), 118 deletions(-) ^ permalink raw reply [nested|flat] 5+ messages in thread
end of thread, other threads:[~2026-08-10 13:41 UTC | newest] Thread overview: 5+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2026-08-10 13:41 pgsql: Teach psql to skip in-line COPY ... FROM STDIN data after a fail Noah Misch <noah@leadboat.com> 2026-08-10 13:41 pgsql: Teach psql to skip in-line COPY ... FROM STDIN data after a fail Noah Misch <noah@leadboat.com> 2026-08-10 13:41 pgsql: Teach psql to skip in-line COPY ... FROM STDIN data after a fail Noah Misch <noah@leadboat.com> 2026-08-10 13:41 pgsql: Teach psql to skip in-line COPY ... FROM STDIN data after a fail Noah Misch <noah@leadboat.com> 2026-08-10 13:41 pgsql: Teach psql to skip in-line COPY ... FROM STDIN data after a fail Noah Misch <noah@leadboat.com>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox