From: Andrew Dunstan Date: Sat, 21 Feb 2026 22:17:48 +0000 (-0500) Subject: Disallow CR and LF in database, role, and tablespace names X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b380a56a3f9;p=thirdparty%2Fpostgresql.git Disallow CR and LF in database, role, and tablespace names Previously, these characters could cause problems when passed through shell commands, and were flagged with a comment in string_utils.c suggesting they be rejected in a future major release. The affected commands are CREATE DATABASE, CREATE ROLE, CREATE TABLESPACE, ALTER DATABASE RENAME, ALTER ROLE RENAME, and ALTER TABLESPACE RENAME. Also add a pg_upgrade check to detect these invalid names in clusters being upgraded from pre-v19 versions, producing a report file listing any offending objects that must be renamed before upgrading. Tests have been modified accordingly. Author: Mahendra Singh Thalor Reviewed-By: Álvaro Herrera Reviewed-By: Andrew Dunstan Reviewed-By: Tom Lane Reviewed-By: Nathan Bossart Reviewed-By: Srinath Reddy Discussion: https://postgr.es/m/CAKYtNApkOi4FY0S7+3jpTqnHVyyZ6Tbzhtbah-NBbY-mGsiKAQ@mail.gmail.com --- diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c index 33311760df7..740c526e92d 100644 --- a/src/backend/commands/dbcommands.c +++ b/src/backend/commands/dbcommands.c @@ -743,6 +743,12 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt) CreateDBStrategy dbstrategy = CREATEDB_WAL_LOG; createdb_failure_params fparms; + /* Report error if name has \n or \r character. */ + if (strpbrk(dbname, "\n\r")) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("database name \"%s\" contains a newline or carriage return character", dbname))); + /* Extract options from the statement node tree */ foreach(option, stmt->options) { @@ -1911,6 +1917,12 @@ RenameDatabase(const char *oldname, const char *newname) int npreparedxacts; ObjectAddress address; + /* Report error if name has \n or \r character. */ + if (strpbrk(newname, "\n\r")) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("database name \"%s\" contains a newline or carriage return character", newname))); + /* * Look up the target database's OID, and get exclusive lock on it. We * need this for the same reasons as DROP DATABASE. diff --git a/src/backend/commands/tablespace.c b/src/backend/commands/tablespace.c index 3511a4ec0fd..ed2a93a09db 100644 --- a/src/backend/commands/tablespace.c +++ b/src/backend/commands/tablespace.c @@ -242,6 +242,12 @@ CreateTableSpace(CreateTableSpaceStmt *stmt) (errcode(ERRCODE_INVALID_NAME), errmsg("tablespace location cannot contain single quotes"))); + /* Report error if name has \n or \r character. */ + if (strpbrk(stmt->tablespacename, "\n\r")) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("tablespace name \"%s\" contains a newline or carriage return character", stmt->tablespacename))); + in_place = allow_in_place_tablespaces && strlen(location) == 0; /* @@ -971,6 +977,12 @@ RenameTableSpace(const char *oldname, const char *newname) errmsg("unacceptable tablespace name \"%s\"", newname), errdetail("The prefix \"pg_\" is reserved for system tablespaces."))); + /* Report error if name has \n or \r character. */ + if (strpbrk(newname, "\n\r")) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("tablespace name \"%s\" contains a newline or carriage return character", newname))); + /* * If built with appropriate switch, whine when regression-testing * conventions for tablespace names are violated. diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c index 8fb9ea25db0..be11c49f919 100644 --- a/src/backend/commands/user.c +++ b/src/backend/commands/user.c @@ -171,6 +171,12 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt) DefElem *dbypassRLS = NULL; GrantRoleOptions popt; + /* Report error if name has \n or \r character. */ + if (strpbrk(stmt->role, "\n\r")) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("role name \"%s\" contains a newline or carriage return character", stmt->role))); + /* The defaults can vary depending on the original statement type */ switch (stmt->stmt_type) { @@ -1348,6 +1354,12 @@ RenameRole(const char *oldname, const char *newname) ObjectAddress address; Form_pg_authid authform; + /* Report error if name has \n or \r character. */ + if (strpbrk(newname, "\n\r")) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("role name \"%s\" contains a newline or carriage return character", newname))); + rel = table_open(AuthIdRelationId, RowExclusiveLock); dsc = RelationGetDescr(rel); diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl index a8dcc2b5c75..bc7a082f57a 100644 --- a/src/bin/pg_dump/t/002_pg_dump.pl +++ b/src/bin/pg_dump/t/002_pg_dump.pl @@ -2048,13 +2048,8 @@ my %tests = ( }, }, - 'newline of role or table name in comment' => { - create_sql => qq{CREATE ROLE regress_newline; - ALTER ROLE regress_newline SET enable_seqscan = off; - ALTER ROLE regress_newline - RENAME TO "regress_newline\nattack"; - - -- meet getPartitioningInfo() "unsafe" condition + 'newline of table name in comment' => { + create_sql => qq{-- meet getPartitioningInfo() "unsafe" condition CREATE TYPE pp_colors AS ENUM ('green', 'blue', 'black'); CREATE TABLE pp_enumpart (a pp_colors) diff --git a/src/bin/pg_dump/t/003_pg_dump_with_server.pl b/src/bin/pg_dump/t/003_pg_dump_with_server.pl index 70f830e10dc..349add67d50 100644 --- a/src/bin/pg_dump/t/003_pg_dump_with_server.pl +++ b/src/bin/pg_dump/t/003_pg_dump_with_server.pl @@ -16,22 +16,6 @@ my $port = $node->port; $node->init; $node->start; -######################################### -# pg_dumpall: newline in database name - -$node->safe_psql('postgres', qq{CREATE DATABASE "regress_\nattack"}); - -my (@cmd, $stdout, $stderr); -@cmd = ("pg_dumpall", '--port' => $port, '--exclude-database=postgres'); -print("# Running: " . join(" ", @cmd) . "\n"); -my $result = IPC::Run::run \@cmd, '>' => \$stdout, '2>' => \$stderr; -ok(!$result, "newline in dbname: exit code not 0"); -like( - $stderr, - qr/shell command argument contains a newline/, - "newline in dbname: stderr matches"); -unlike($stdout, qr/^attack/m, "newline in dbname: no comment escape"); - ######################################### # Verify that dumping foreign data includes only foreign tables of # matching servers diff --git a/src/bin/pg_dump/t/010_dump_connstr.pl b/src/bin/pg_dump/t/010_dump_connstr.pl index dc7a33658db..bf2c3b6d00b 100644 --- a/src/bin/pg_dump/t/010_dump_connstr.pl +++ b/src/bin/pg_dump/t/010_dump_connstr.pl @@ -153,20 +153,6 @@ $node->command_ok( ], 'pg_dumpall --dbname accepts connection string'); -$node->run_log( - [ 'createdb', '--username' => $src_bootstrap_super, "foo\n\rbar" ]); - -# not sufficient to use --roles-only here -$node->command_fails( - [ - 'pg_dumpall', '--no-sync', - '--username' => $src_bootstrap_super, - '--file' => $discard, - ], - 'pg_dumpall with \n\r in database name'); -$node->run_log( - [ 'dropdb', '--username' => $src_bootstrap_super, "foo\n\rbar" ]); - # make a table, so the parallel worker has something to dump $node->safe_psql( diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c index 5c73773bf0e..c0e7ec35168 100644 --- a/src/bin/pg_upgrade/check.c +++ b/src/bin/pg_upgrade/check.c @@ -34,6 +34,7 @@ static void check_new_cluster_replication_slots(void); static void check_new_cluster_subscription_configuration(void); static void check_old_cluster_for_valid_slots(void); static void check_old_cluster_subscription_state(void); +static void check_old_cluster_global_names(ClusterInfo *cluster); /* * DataTypesUsageChecks - definitions of data type checks for the old cluster @@ -600,6 +601,14 @@ check_and_dump_old_cluster(void) */ check_for_connection_status(&old_cluster); + /* + * Validate database, user, role and tablespace names from the old cluster. + * No need to check in 19 or newer as newline and carriage return are + * not allowed at the creation time of the object. + */ + if (GET_MAJOR_VERSION(old_cluster.major_version) < 1900) + check_old_cluster_global_names(&old_cluster); + /* * Extract a list of databases, tables, and logical replication slots from * the old cluster. @@ -2500,3 +2509,73 @@ check_old_cluster_subscription_state(void) else check_ok(); } + +/* + * check_old_cluster_global_names() + * + * Raise an error if any database, role, or tablespace name contains a newline + * or carriage return character. Such names are not allowed in v19 and later. + */ +static void +check_old_cluster_global_names(ClusterInfo *cluster) +{ + int i; + PGconn *conn_template1; + PGresult *res; + int ntups; + FILE *script = NULL; + char output_path[MAXPGPATH]; + int count = 0; + + prep_status("Checking names of databases, roles and tablespaces"); + + snprintf(output_path, sizeof(output_path), "%s/%s", + log_opts.basedir, + "db_role_tablespace_invalid_names.txt"); + + conn_template1 = connectToServer(cluster, "template1"); + + /* + * Get database, user/role and tablespacenames from cluster. Can't use + * pg_authid because only superusers can view it. + */ + res = executeQueryOrDie(conn_template1, + "SELECT datname AS objname, 'database' AS objtype " + "FROM pg_catalog.pg_database UNION ALL " + "SELECT rolname AS objname, 'role' AS objtype " + "FROM pg_catalog.pg_roles UNION ALL " + "SELECT spcname AS objname, 'tablespace' AS objtype " + "FROM pg_catalog.pg_tablespace ORDER BY 2 "); + + ntups = PQntuples(res); + for (i = 0; i < ntups; i++) + { + char *objname = PQgetvalue(res, i, 0); + char *objtype = PQgetvalue(res, i, 1); + + /* If name has \n or \r, then report it. */ + if (strpbrk(objname, "\n\r")) + { + if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL) + pg_fatal("could not open file \"%s\": %m", output_path); + + fprintf(script, "%d : %s name = \"%s\"\n", ++count, objtype, objname); + } + } + + PQclear(res); + PQfinish(conn_template1); + + if (script) + { + fclose(script); + pg_log(PG_REPORT, "fatal"); + pg_fatal("All the database, role and tablespace names should have only valid characters. A newline or \n" + "carriage return character is not allowed in these object names. To fix this, please \n" + "rename these names with valid names. \n" + "To see all %d invalid object names, refer db_role_tablespace_invalid_names.txt file. \n" + " %s", count, output_path); + } + else + check_ok(); +} diff --git a/src/bin/scripts/t/020_createdb.pl b/src/bin/scripts/t/020_createdb.pl index 83b0077383a..a0995868363 100644 --- a/src/bin/scripts/t/020_createdb.pl +++ b/src/bin/scripts/t/020_createdb.pl @@ -241,6 +241,18 @@ $node->command_fails( ], 'fails for invalid locale provider'); +$node->command_fails_like( + [ 'createdb', "invalid \n dbname" ], + qr(contains a newline or carriage return character), + 'fails if database name contains a newline character in name' +); + +$node->command_fails_like( + [ 'createdb', "invalid \r dbname" ], + qr(contains a newline or carriage return character), + 'fails if database name contains a carriage return character in name' +); + # Check use of templates with shared dependencies copied from the template. my ($ret, $stdout, $stderr) = $node->psql( 'foobar2', diff --git a/src/fe_utils/string_utils.c b/src/fe_utils/string_utils.c index 89ce396ed4f..38fffbd036b 100644 --- a/src/fe_utils/string_utils.c +++ b/src/fe_utils/string_utils.c @@ -568,12 +568,6 @@ appendByteaLiteral(PQExpBuffer buf, const unsigned char *str, size_t length, * Append the given string to the shell command being built in the buffer, * with shell-style quoting as needed to create exactly one argument. * - * Forbid LF or CR characters, which have scant practical use beyond designing - * security breaches. The Windows command shell is unusable as a conduit for - * arguments containing LF or CR characters. A future major release should - * reject those characters in CREATE ROLE and CREATE DATABASE, because use - * there eventually leads to errors here. - * * appendShellString() simply prints an error and dies if LF or CR appears. * appendShellStringNoError() omits those characters from the result, and * returns false if there were any. diff --git a/src/test/modules/unsafe_tests/expected/alter_system_table.out b/src/test/modules/unsafe_tests/expected/alter_system_table.out index b73b9442b8d..9ea6061f4ae 100644 --- a/src/test/modules/unsafe_tests/expected/alter_system_table.out +++ b/src/test/modules/unsafe_tests/expected/alter_system_table.out @@ -64,6 +64,11 @@ ERROR: permission denied: "pg_description" is a system catalog CREATE TABLESPACE pg_foo LOCATION '/no/such/location'; ERROR: unacceptable tablespace name "pg_foo" DETAIL: The prefix "pg_" is reserved for system tablespaces. +-- contains \n\r tablespace name +CREATE TABLESPACE "invalid +name" LOCATION '/no/such/location'; +ERROR: tablespace name "invalid +name" contains a newline or carriage return character -- triggers CREATE FUNCTION tf1() RETURNS trigger LANGUAGE plpgsql diff --git a/src/test/modules/unsafe_tests/expected/rolenames.out b/src/test/modules/unsafe_tests/expected/rolenames.out index 61396b2a805..9dfb8475e16 100644 --- a/src/test/modules/unsafe_tests/expected/rolenames.out +++ b/src/test/modules/unsafe_tests/expected/rolenames.out @@ -102,6 +102,10 @@ DETAIL: Role names starting with "pg_" are reserved. CREATE ROLE "pg_abcdef"; -- error ERROR: role name "pg_abcdef" is reserved DETAIL: Role names starting with "pg_" are reserved. +CREATE ROLE "invalid +rolename"; -- error +ERROR: role name "invalid +rolename" contains a newline or carriage return character CREATE ROLE regress_testrol0 SUPERUSER LOGIN; CREATE ROLE regress_testrolx SUPERUSER LOGIN; CREATE ROLE regress_testrol2 SUPERUSER; diff --git a/src/test/modules/unsafe_tests/sql/alter_system_table.sql b/src/test/modules/unsafe_tests/sql/alter_system_table.sql index c1515100845..1b0eb727eaa 100644 --- a/src/test/modules/unsafe_tests/sql/alter_system_table.sql +++ b/src/test/modules/unsafe_tests/sql/alter_system_table.sql @@ -64,6 +64,10 @@ ALTER TABLE pg_description SET SCHEMA public; -- reserved tablespace name CREATE TABLESPACE pg_foo LOCATION '/no/such/location'; +-- contains \n\r tablespace name +CREATE TABLESPACE "invalid +name" LOCATION '/no/such/location'; + -- triggers CREATE FUNCTION tf1() RETURNS trigger LANGUAGE plpgsql diff --git a/src/test/modules/unsafe_tests/sql/rolenames.sql b/src/test/modules/unsafe_tests/sql/rolenames.sql index adac36536db..70b342abeb6 100644 --- a/src/test/modules/unsafe_tests/sql/rolenames.sql +++ b/src/test/modules/unsafe_tests/sql/rolenames.sql @@ -75,6 +75,8 @@ CREATE ROLE pg_abc; -- error CREATE ROLE "pg_abc"; -- error CREATE ROLE pg_abcdef; -- error CREATE ROLE "pg_abcdef"; -- error +CREATE ROLE "invalid +rolename"; -- error CREATE ROLE regress_testrol0 SUPERUSER LOGIN; CREATE ROLE regress_testrolx SUPERUSER LOGIN; diff --git a/src/test/regress/expected/tablespace.out b/src/test/regress/expected/tablespace.out index a90e39e5738..185880a3217 100644 --- a/src/test/regress/expected/tablespace.out +++ b/src/test/regress/expected/tablespace.out @@ -958,6 +958,11 @@ ALTER TABLE ALL IN TABLESPACE regress_tblspace_renamed SET TABLESPACE pg_default NOTICE: no matching relations in tablespace "regress_tblspace_renamed" found ALTER MATERIALIZED VIEW ALL IN TABLESPACE regress_tblspace_renamed SET TABLESPACE pg_default; NOTICE: no matching relations in tablespace "regress_tblspace_renamed" found +-- Should fail, contains \n in name +ALTER TABLESPACE regress_tblspace_renamed RENAME TO "invalid +name"; +ERROR: tablespace name "invalid +name" contains a newline or carriage return character -- Should succeed DROP TABLESPACE regress_tblspace_renamed; DROP SCHEMA testschema CASCADE; diff --git a/src/test/regress/sql/tablespace.sql b/src/test/regress/sql/tablespace.sql index dfe3db096e2..c43a59e5957 100644 --- a/src/test/regress/sql/tablespace.sql +++ b/src/test/regress/sql/tablespace.sql @@ -430,6 +430,10 @@ ALTER MATERIALIZED VIEW ALL IN TABLESPACE regress_tblspace_renamed SET TABLESPAC ALTER TABLE ALL IN TABLESPACE regress_tblspace_renamed SET TABLESPACE pg_default; ALTER MATERIALIZED VIEW ALL IN TABLESPACE regress_tblspace_renamed SET TABLESPACE pg_default; +-- Should fail, contains \n in name +ALTER TABLESPACE regress_tblspace_renamed RENAME TO "invalid +name"; + -- Should succeed DROP TABLESPACE regress_tblspace_renamed;