From 016a8168b39b67a6468ee1a752ee9ec82cf3fecb Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Wed, 7 Sep 2016 16:39:57 +0900 Subject: [PATCH 1/2] Forbid newline and carriage return characters in database and role names --- doc/src/sgml/ref/create_database.sgml | 7 +++++++ doc/src/sgml/ref/create_role.sgml | 7 +++++++ src/backend/commands/dbcommands.c | 23 +++++++++++++++++++++++ src/backend/commands/user.c | 21 +++++++++++++++++++++ src/fe_utils/string_utils.c | 4 +--- 5 files changed, 59 insertions(+), 3 deletions(-) diff --git a/doc/src/sgml/ref/create_database.sgml b/doc/src/sgml/ref/create_database.sgml index cf33746..2477ba7 100644 --- a/doc/src/sgml/ref/create_database.sgml +++ b/doc/src/sgml/ref/create_database.sgml @@ -260,6 +260,13 @@ CREATE DATABASE name connection slot remains for the database, it is possible that both will fail. Also, the limit is not enforced against superusers. + + + Database names cannot include newline or carriage return characters + as those could be at the origin of security breaches, particularly on + Windows where the command shell is unusable with arguments containing + such characters. + diff --git a/doc/src/sgml/ref/create_role.sgml b/doc/src/sgml/ref/create_role.sgml index 38cd4c8..9d6926e 100644 --- a/doc/src/sgml/ref/create_role.sgml +++ b/doc/src/sgml/ref/create_role.sgml @@ -404,6 +404,13 @@ CREATE ROLE name [ [ WITH ] \password that can be used to safely change the password later. + + + Role names cannot include newline or carriage return characters + as those could be at the origin of security breaches, particularly on + Windows where the command shell is unusable with arguments containing + such characters. + diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c index ef48659..22712ec 100644 --- a/src/backend/commands/dbcommands.c +++ b/src/backend/commands/dbcommands.c @@ -77,6 +77,7 @@ typedef struct } movedb_failure_params; /* non-export function prototypes */ +static void check_db_name(const char *dbname); static void createdb_failure_callback(int code, Datum arg); static void movedb(const char *dbname, const char *tblspcname); static void movedb_failure_callback(int code, Datum arg); @@ -469,6 +470,9 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt) /* Note there is no additional permission check in this path */ } + /* do sanity checks on database name */ + check_db_name(dbname); + /* * Check for db name conflict. This is just to give a more friendly error * message than "unique index violation". There's a race condition but @@ -758,6 +762,22 @@ check_encoding_locale_matches(int encoding, const char *collate, const char *cty pg_encoding_to_char(collate_encoding)))); } +/* + * Perform sanity checks on the database name. + */ +static void +check_db_name(const char *dbname) +{ + if (strchr(dbname, '\n') != NULL) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("database name cannot include newline character"))); + if (strchr(dbname, '\r') != NULL) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("database name cannot include carriage return character"))); +} + /* Error cleanup callback for createdb */ static void createdb_failure_callback(int code, Datum arg) @@ -962,6 +982,9 @@ RenameDatabase(const char *oldname, const char *newname) int npreparedxacts; ObjectAddress address; + /* check format of new name */ + check_db_name(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/user.c b/src/backend/commands/user.c index 4027c89..fd737d8 100644 --- a/src/backend/commands/user.c +++ b/src/backend/commands/user.c @@ -57,6 +57,21 @@ static void DelRoleMems(const char *rolename, Oid roleid, bool admin_opt); +/* Do sanity checks on role name */ +static void +check_role_name(const char *rolname) +{ + if (strchr(rolname, '\n') != NULL) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("role name cannot use newline character"))); + if (strchr(rolname, '\r') != NULL) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("role name cannot use carriage return character"))); +} + + /* Check if current user has createrole privileges */ static bool have_createrole_privilege(void) @@ -111,6 +126,9 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt) DefElem *dvalidUntil = NULL; DefElem *dbypassRLS = NULL; + /* sanity check for role name */ + check_role_name(stmt->role); + /* The defaults can vary depending on the original statement type */ switch (stmt->stmt_type) { @@ -1150,6 +1168,9 @@ RenameRole(const char *oldname, const char *newname) ObjectAddress address; Form_pg_authid authform; + /* sanity check for role name */ + check_role_name(newname); + rel = heap_open(AuthIdRelationId, RowExclusiveLock); dsc = RelationGetDescr(rel); diff --git a/src/fe_utils/string_utils.c b/src/fe_utils/string_utils.c index 61bf9e6..1c3d9b3 100644 --- a/src/fe_utils/string_utils.c +++ b/src/fe_utils/string_utils.c @@ -422,9 +422,7 @@ appendByteaLiteral(PQExpBuffer buf, const unsigned char *str, size_t length, * * 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. + * arguments containing LF or CR characters. */ void appendShellString(PQExpBuffer buf, const char *str) -- 2.10.0