On Fri, Sep 2, 2016 at 2:44 AM, Peter Eisentraut
<[email protected]> wrote:
> On 8/11/16 9:12 PM, Michael Paquier wrote:
>> Note that pg_dump[all] and pg_upgrade already have safeguards against
>> those things per the same routines putting quotes for execution as
>> commands into psql and shell. So attached is a patch to implement this
>> restriction in the backend,
>
> How about some documentation? I think the CREATE ROLE and CREATE
> DATABASE man pages might be suitable places.
Sure. What do you think about that?
+ <para>
+ Database names cannot include <literal>LF</> or <literal>CR</> 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.
+ </para>
--
Michael
diff --git a/doc/src/sgml/ref/create_database.sgml b/doc/src/sgml/ref/create_database.sgml
index cf33746..d73dc3a 100644
--- a/doc/src/sgml/ref/create_database.sgml
+++ b/doc/src/sgml/ref/create_database.sgml
@@ -260,6 +260,13 @@ CREATE DATABASE <replaceable class="PARAMETER">name</replaceable>
connection <quote>slot</> remains for the database, it is possible that
both will fail. Also, the limit is not enforced against superusers.
</para>
+
+ <para>
+ Database names cannot include <literal>LF</> or <literal>CR</> 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.
+ </para>
</refsect1>
<refsect1>
diff --git a/doc/src/sgml/ref/create_role.sgml b/doc/src/sgml/ref/create_role.sgml
index 38cd4c8..27259f2 100644
--- a/doc/src/sgml/ref/create_role.sgml
+++ b/doc/src/sgml/ref/create_role.sgml
@@ -404,6 +404,13 @@ CREATE ROLE <replaceable class="PARAMETER">name</replaceable> [ [ WITH ] <replac
<command>\password</command> that can be used to safely change the
password later.
</para>
+
+ <para>
+ Role names cannot include <literal>LF</> or <literal>CR</> 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.
+ <para>
</refsect1>
<refsect1>
diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c
index c1c0223..5746958 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);
@@ -456,6 +457,9 @@ createdb(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
@@ -745,6 +749,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 use LF character")));
+ if (strchr(dbname, '\r') != NULL)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("database name cannot use CR character")));
+}
+
/* Error cleanup callback for createdb */
static void
createdb_failure_callback(int code, Datum arg)
@@ -949,6 +969,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 b6ea950..8954e16 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 LF character")));
+ if (strchr(rolname, '\r') != NULL)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("role name cannot use CR character")));
+}
+
+
/* Check if current user has createrole privileges */
static bool
have_createrole_privilege(void)
@@ -111,6 +126,9 @@ CreateRole(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)
{
@@ -1137,6 +1155,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);
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
