From e7b269675c27a8cd2e785b8bd22a163e1dcaee95 Mon Sep 17 00:00:00 2001 From: Mahendra Singh Thalor Date: Fri, 28 Mar 2025 00:20:15 +0530 Subject: [PATCH] don't allow newline or carriage return character in name for database/user/role While creating database, if database name has any newline or carriage return character in name, then through error becuase these special character are not allowed in dbname when dump command is executed. do same for "CREATE ROLE", "CREATE USER" also. This will add check for: "CREATE DATABASE", "CREATE ROLE", "CREATE USER", "RENAME DATABASE/USER/ROLE" ------------------------- As we will not allow these \n\r in names, then we will never get these names in dump also. If we are dumping from older branch, then we will fail with same old error. (dump will fail in older branches so no need to add extra handling for dump.) Also remove comment added by 142c24c23447f212e642a0ffac9af878b93f490d commit. Remove one test of 8b845520fb0aa50fea7aae44a45cee1b6d87845d commit. --- src/backend/commands/dbcommands.c | 14 ++++++++++++++ src/backend/commands/user.c | 14 ++++++++++++++ src/bin/pg_dump/t/010_dump_connstr.pl | 14 -------------- src/bin/scripts/t/020_createdb.pl | 12 ++++++++++++ src/bin/scripts/t/040_createuser.pl | 4 ++++ src/fe_utils/string_utils.c | 6 ------ 6 files changed, 44 insertions(+), 20 deletions(-) mode change 100644 => 100755 src/bin/pg_dump/t/010_dump_connstr.pl mode change 100644 => 100755 src/bin/scripts/t/020_createdb.pl mode change 100644 => 100755 src/bin/scripts/t/040_createuser.pl diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c index 5fbbcdaabb1..c8145b0b6e6 100644 --- a/src/backend/commands/dbcommands.c +++ b/src/backend/commands/dbcommands.c @@ -741,6 +741,13 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt) CreateDBStrategy dbstrategy = CREATEDB_WAL_LOG; createdb_failure_params fparms; + /* Report error if dbname have newline or carriage return in name. */ + if (strpbrk(dbname, "\n\r")) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE)), + errmsg("database name contains a newline or carriage return character"), + errhint("newline or carriage return character is not allowed in database name")); + /* Extract options from the statement node tree */ foreach(option, stmt->options) { @@ -1884,6 +1891,13 @@ RenameDatabase(const char *oldname, const char *newname) int npreparedxacts; ObjectAddress address; + /* Report error if dbname have newline or carriage return in name. */ + if (strpbrk(newname, "\n\r")) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE)), + errmsg("database new name contains a newline or carriage return character"), + errhint("newline or carriage return character is not allowed in database name")); + /* * 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/user.c b/src/backend/commands/user.c index 8ae510c623b..6a103a22197 100644 --- a/src/backend/commands/user.c +++ b/src/backend/commands/user.c @@ -171,6 +171,13 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt) DefElem *dbypassRLS = NULL; GrantRoleOptions popt; + /* Report error if role name has newline or carriage return in name. */ + if (strpbrk(stmt->role, "\n\r")) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE)), + errmsg("role name contains a newline or carriage return character"), + errhint("newline or carriage return character is not allowed in role name")); + /* The defaults can vary depending on the original statement type */ switch (stmt->stmt_type) { @@ -1347,6 +1354,13 @@ RenameRole(const char *oldname, const char *newname) ObjectAddress address; Form_pg_authid authform; + /* Report error if role name has newline or carriage return in name. */ + if (strpbrk(newname, "\n\r")) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE)), + errmsg("role name contains a newline or carriage return character"), + errhint("newline or carriage return character is not allowed in role name")); + rel = table_open(AuthIdRelationId, RowExclusiveLock); dsc = RelationGetDescr(rel); diff --git a/src/bin/pg_dump/t/010_dump_connstr.pl b/src/bin/pg_dump/t/010_dump_connstr.pl old mode 100644 new mode 100755 index bde6096c60d..5054224fa12 --- 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/scripts/t/020_createdb.pl b/src/bin/scripts/t/020_createdb.pl old mode 100644 new mode 100755 index a8293390ede..4cc3ad0f5cd --- 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/ERROR: database name contains a newline or carriage return character/, + 'fails if database name containing newline character in name' +); + +$node->command_fails_like( + [ 'createdb', "invalid \r dbname" ], + qr/ERROR: database name contains a newline or carriage return character/, + 'fails if database name containing 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/bin/scripts/t/040_createuser.pl b/src/bin/scripts/t/040_createuser.pl old mode 100644 new mode 100755 index 54af43401bb..fd1b3f83899 --- a/src/bin/scripts/t/040_createuser.pl +++ b/src/bin/scripts/t/040_createuser.pl @@ -89,5 +89,9 @@ $node->command_fails( 'regress_user3' ], 'fails for too many non-options'); +$node->command_fails([ 'createuser', "invalid \n username" ], + 'fails as newline is not allowed in role name'); +$node->command_fails([ 'createuser', "invalid \r username" ], + 'fails as carraige return is not allowed in role name'); done_testing(); diff --git a/src/fe_utils/string_utils.c b/src/fe_utils/string_utils.c index 130d1020d50..6f25b5286ed 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. -- 2.39.3