Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Fix crashes with CREATE SCHEMA AUTHORIZATION and schema elements
authorMichael Paquier <michael@paquier.xyz>
Fri, 28 Apr 2023 10:29:40 +0000 (19:29 +0900)
committerMichael Paquier <michael@paquier.xyz>
Fri, 28 Apr 2023 10:29:40 +0000 (19:29 +0900)
CREATE SCHEMA AUTHORIZATION with appended schema elements can lead to
crashes when comparing the schema name of the query with the schemas
used in the qualification of some clauses in the elements' queries.

The origin of the problem is that the transformation routine for the
elements listed in a CREATE SCHEMA query uses as new, expected, schema
name the one listed in CreateSchemaStmt itself.  However, depending on
the query, CreateSchemaStmt.schemaname may be NULL, being computed
instead from the role specification of the query given by the
AUTHORIZATION clause, that could be either:
- A user name string, with the new schema name being set to the same
value as the role given.
- Guessed from CURRENT_ROLE, SESSION_ROLE or CURRENT_ROLE, with a new
schema name computed from the security context where CREATE SCHEMA is
running.

Regression tests are added for CREATE SCHEMA with some appended elements
(some of them with schema qualifications), covering also some role
specification patterns.

While on it, this simplifies the context structure used during the
transformation of the elements listed in a CREATE SCHEMA query by
removing the fields for the role specification and the role type.  They
were not used, and for the role specification this could be confusing as
the schema name may by extracted from that at the beginning of
CreateSchemaCommand().

This issue exists for a long time, so backpatch down to all the versions
supported.

Reported-by: Song Hongyu
Author: Michael Paquier
Reviewed-by: Richard Guo
Discussion: https://postgr.es/m/17909-f65c12dfc5f0451d@postgresql.org
Backpatch-through: 11

src/backend/commands/schemacmds.c
src/backend/parser/parse_utilcmd.c
src/include/parser/parse_utilcmd.h
src/test/regress/expected/create_schema.out [new file with mode: 0644]
src/test/regress/parallel_schedule
src/test/regress/serial_schedule
src/test/regress/sql/create_schema.sql [new file with mode: 0644]

index 3ccd743118fc8f21647e66507d369f422898e434..15f4d707885948e694a1a50aa4639f6022875ebb 100644 (file)
@@ -178,7 +178,8 @@ CreateSchemaCommand(CreateSchemaStmt *stmt, const char *queryString,
     * we cannot, in general, run parse analysis on one statement until we
     * have actually executed the prior ones.
     */
-   parsetree_list = transformCreateSchemaStmt(stmt);
+   parsetree_list = transformCreateSchemaStmtElements(stmt->schemaElts,
+                                                      schemaName);
 
    /*
     * Execute each command contained in the CREATE SCHEMA.  Since the grammar
index 8b91453dd54bddc4fa9043933b983d3ee46b21f1..76ba78459a7c79e780c48e196f8053a3b1a30a29 100644 (file)
@@ -98,12 +98,10 @@ typedef struct
    bool        ofType;         /* true if statement contains OF typename */
 } CreateStmtContext;
 
-/* State shared by transformCreateSchemaStmt and its subroutines */
+/* State shared by transformCreateSchemaStmtElements and its subroutines */
 typedef struct
 {
-   const char *stmtType;       /* "CREATE SCHEMA" or "ALTER SCHEMA" */
-   char       *schemaname;     /* name of schema */
-   RoleSpec   *authrole;       /* owner of schema */
+   const char *schemaname;     /* name of schema */
    List       *sequences;      /* CREATE SEQUENCE items */
    List       *tables;         /* CREATE TABLE items */
    List       *views;          /* CREATE VIEW items */
@@ -137,7 +135,7 @@ static void transformCheckConstraints(CreateStmtContext *cxt,
 static void transformConstraintAttrs(CreateStmtContext *cxt,
                                     List *constraintList);
 static void transformColumnType(CreateStmtContext *cxt, ColumnDef *column);
-static void setSchemaName(char *context_schema, char **stmt_schema_name);
+static void setSchemaName(const char *context_schema, char **stmt_schema_name);
 static void transformPartitionCmd(CreateStmtContext *cxt, PartitionCmd *cmd);
 static List *transformPartitionRangeBounds(ParseState *pstate, List *blist,
                                           Relation parent);
@@ -3706,14 +3704,18 @@ transformColumnType(CreateStmtContext *cxt, ColumnDef *column)
 
 
 /*
- * transformCreateSchemaStmt -
- *   analyzes the CREATE SCHEMA statement
+ * transformCreateSchemaStmtElements -
+ *   analyzes the elements of a CREATE SCHEMA statement
  *
- * Split the schema element list into individual commands and place
- * them in the result list in an order such that there are no forward
- * references (e.g. GRANT to a table created later in the list). Note
- * that the logic we use for determining forward references is
- * presently quite incomplete.
+ * Split the schema element list from a CREATE SCHEMA statement into
+ * individual commands and place them in the result list in an order
+ * such that there are no forward references (e.g. GRANT to a table
+ * created later in the list). Note that the logic we use for determining
+ * forward references is presently quite incomplete.
+ *
+ * "schemaName" is the name of the schema that will be used for the creation
+ * of the objects listed, that may be compiled from the schema name defined
+ * in the statement or a role specification.
  *
  * SQL also allows constraints to make forward references, so thumb through
  * the table columns and move forward references to a posterior alter-table
@@ -3729,15 +3731,13 @@ transformColumnType(CreateStmtContext *cxt, ColumnDef *column)
  * extent.
  */
 List *
-transformCreateSchemaStmt(CreateSchemaStmt *stmt)
+transformCreateSchemaStmtElements(List *schemaElts, const char *schemaName)
 {
    CreateSchemaStmtContext cxt;
    List       *result;
    ListCell   *elements;
 
-   cxt.stmtType = "CREATE SCHEMA";
-   cxt.schemaname = stmt->schemaname;
-   cxt.authrole = (RoleSpec *) stmt->authrole;
+   cxt.schemaname = schemaName;
    cxt.sequences = NIL;
    cxt.tables = NIL;
    cxt.views = NIL;
@@ -3749,7 +3749,7 @@ transformCreateSchemaStmt(CreateSchemaStmt *stmt)
     * Run through each schema element in the schema element list. Separate
     * statements by type, and do preliminary analysis.
     */
-   foreach(elements, stmt->schemaElts)
+   foreach(elements, schemaElts)
    {
        Node       *element = lfirst(elements);
 
@@ -3834,10 +3834,10 @@ transformCreateSchemaStmt(CreateSchemaStmt *stmt)
  *     Set or check schema name in an element of a CREATE SCHEMA command
  */
 static void
-setSchemaName(char *context_schema, char **stmt_schema_name)
+setSchemaName(const char *context_schema, char **stmt_schema_name)
 {
    if (*stmt_schema_name == NULL)
-       *stmt_schema_name = context_schema;
+       *stmt_schema_name = unconstify(char *, context_schema);
    else if (strcmp(context_schema, *stmt_schema_name) != 0)
        ereport(ERROR,
                (errcode(ERRCODE_INVALID_SCHEMA_DEFINITION),
index bc3d66ed88146cbd4a1271a7c05fe16e505329e1..305ce124a8bc5e6d879dd7902d168315df6e0cbd 100644 (file)
@@ -28,7 +28,8 @@ extern IndexStmt *transformIndexStmt(Oid relid, IndexStmt *stmt,
                                     const char *queryString);
 extern void transformRuleStmt(RuleStmt *stmt, const char *queryString,
                              List **actions, Node **whereClause);
-extern List *transformCreateSchemaStmt(CreateSchemaStmt *stmt);
+extern List *transformCreateSchemaStmtElements(List *schemaElts,
+                                              const char *schemaName);
 extern PartitionBoundSpec *transformPartitionBound(ParseState *pstate, Relation parent,
                                                   PartitionBoundSpec *spec);
 extern List *expandTableLikeClause(RangeVar *heapRel,
diff --git a/src/test/regress/expected/create_schema.out b/src/test/regress/expected/create_schema.out
new file mode 100644 (file)
index 0000000..691f545
--- /dev/null
@@ -0,0 +1,98 @@
+--
+-- CREATE_SCHEMA
+--
+-- Schema creation with elements.
+CREATE ROLE regress_create_schema_role SUPERUSER;
+-- Cases where schema creation fails as objects are qualified with a schema
+-- that does not match with what's expected.
+-- This checks all the object types that include schema qualifications.
+CREATE SCHEMA AUTHORIZATION regress_create_schema_role
+  CREATE SEQUENCE schema_not_existing.seq;
+ERROR:  CREATE specifies a schema (schema_not_existing) different from the one being created (regress_create_schema_role)
+CREATE SCHEMA AUTHORIZATION regress_create_schema_role
+  CREATE TABLE schema_not_existing.tab (id int);
+ERROR:  CREATE specifies a schema (schema_not_existing) different from the one being created (regress_create_schema_role)
+CREATE SCHEMA AUTHORIZATION regress_create_schema_role
+  CREATE VIEW schema_not_existing.view AS SELECT 1;
+ERROR:  CREATE specifies a schema (schema_not_existing) different from the one being created (regress_create_schema_role)
+CREATE SCHEMA AUTHORIZATION regress_create_schema_role
+  CREATE INDEX ON schema_not_existing.tab (id);
+ERROR:  CREATE specifies a schema (schema_not_existing) different from the one being created (regress_create_schema_role)
+CREATE SCHEMA AUTHORIZATION regress_create_schema_role
+  CREATE TRIGGER schema_trig BEFORE INSERT ON schema_not_existing.tab
+  EXECUTE FUNCTION schema_trig.no_func();
+ERROR:  CREATE specifies a schema (schema_not_existing) different from the one being created (regress_create_schema_role)
+-- Again, with a role specification and no schema names.
+SET ROLE regress_create_schema_role;
+CREATE SCHEMA AUTHORIZATION CURRENT_USER
+  CREATE SEQUENCE schema_not_existing.seq;
+ERROR:  CREATE specifies a schema (schema_not_existing) different from the one being created (regress_create_schema_role)
+CREATE SCHEMA AUTHORIZATION CURRENT_USER
+  CREATE TABLE schema_not_existing.tab (id int);
+ERROR:  CREATE specifies a schema (schema_not_existing) different from the one being created (regress_create_schema_role)
+CREATE SCHEMA AUTHORIZATION CURRENT_USER
+  CREATE VIEW schema_not_existing.view AS SELECT 1;
+ERROR:  CREATE specifies a schema (schema_not_existing) different from the one being created (regress_create_schema_role)
+CREATE SCHEMA AUTHORIZATION CURRENT_USER
+  CREATE INDEX ON schema_not_existing.tab (id);
+ERROR:  CREATE specifies a schema (schema_not_existing) different from the one being created (regress_create_schema_role)
+CREATE SCHEMA AUTHORIZATION CURRENT_USER
+  CREATE TRIGGER schema_trig BEFORE INSERT ON schema_not_existing.tab
+  EXECUTE FUNCTION schema_trig.no_func();
+ERROR:  CREATE specifies a schema (schema_not_existing) different from the one being created (regress_create_schema_role)
+-- Again, with a schema name and a role specification.
+CREATE SCHEMA regress_schema_1 AUTHORIZATION CURRENT_USER
+  CREATE SEQUENCE schema_not_existing.seq;
+ERROR:  CREATE specifies a schema (schema_not_existing) different from the one being created (regress_schema_1)
+CREATE SCHEMA regress_schema_1 AUTHORIZATION CURRENT_USER
+  CREATE TABLE schema_not_existing.tab (id int);
+ERROR:  CREATE specifies a schema (schema_not_existing) different from the one being created (regress_schema_1)
+CREATE SCHEMA regress_schema_1 AUTHORIZATION CURRENT_USER
+  CREATE VIEW schema_not_existing.view AS SELECT 1;
+ERROR:  CREATE specifies a schema (schema_not_existing) different from the one being created (regress_schema_1)
+CREATE SCHEMA regress_schema_1 AUTHORIZATION CURRENT_USER
+  CREATE INDEX ON schema_not_existing.tab (id);
+ERROR:  CREATE specifies a schema (schema_not_existing) different from the one being created (regress_schema_1)
+CREATE SCHEMA regress_schema_1 AUTHORIZATION CURRENT_USER
+  CREATE TRIGGER schema_trig BEFORE INSERT ON schema_not_existing.tab
+  EXECUTE FUNCTION schema_trig.no_func();
+ERROR:  CREATE specifies a schema (schema_not_existing) different from the one being created (regress_schema_1)
+RESET ROLE;
+-- Cases where the schema creation succeeds.
+-- The schema created matches the role name.
+CREATE SCHEMA AUTHORIZATION regress_create_schema_role
+  CREATE TABLE regress_create_schema_role.tab (id int);
+\d regress_create_schema_role.tab
+      Table "regress_create_schema_role.tab"
+ Column |  Type   | Collation | Nullable | Default 
+--------+---------+-----------+----------+---------
+ id     | integer |           |          | 
+
+DROP SCHEMA regress_create_schema_role CASCADE;
+NOTICE:  drop cascades to table regress_create_schema_role.tab
+-- Again, with a different role specification and no schema names.
+SET ROLE regress_create_schema_role;
+CREATE SCHEMA AUTHORIZATION CURRENT_USER
+  CREATE TABLE regress_create_schema_role.tab (id int);
+\d regress_create_schema_role.tab
+      Table "regress_create_schema_role.tab"
+ Column |  Type   | Collation | Nullable | Default 
+--------+---------+-----------+----------+---------
+ id     | integer |           |          | 
+
+DROP SCHEMA regress_create_schema_role CASCADE;
+NOTICE:  drop cascades to table tab
+-- Again, with a schema name and a role specification.
+CREATE SCHEMA regress_schema_1 AUTHORIZATION CURRENT_USER
+  CREATE TABLE regress_schema_1.tab (id int);
+\d regress_schema_1.tab
+           Table "regress_schema_1.tab"
+ Column |  Type   | Collation | Nullable | Default 
+--------+---------+-----------+----------+---------
+ id     | integer |           |          | 
+
+DROP SCHEMA regress_schema_1 CASCADE;
+NOTICE:  drop cascades to table regress_schema_1.tab
+RESET ROLE;
+-- Clean up
+DROP ROLE regress_create_schema_role;
index ae89ed7f0b40bd0a0213bdfef8b259074477220c..ddb34b920e763b246ab0b11a90be20b461e8d5c4 100644 (file)
@@ -48,7 +48,7 @@ test: copy copyselect copydml insert insert_conflict
 # ----------
 # More groups of parallel tests
 # ----------
-test: create_misc create_operator create_procedure
+test: create_misc create_operator create_procedure create_schema
 # These depend on create_misc and create_operator
 test: create_index create_index_spgist create_view index_including index_including_gist
 
index 525bdc804f61286dd4baccf627ef8c8dea0e8fe0..42d78f4f631b970070c0d79ef8e19d5163f92b67 100644 (file)
@@ -63,6 +63,7 @@ test: insert_conflict
 test: create_misc
 test: create_operator
 test: create_procedure
+test: create_schema
 test: create_index
 test: create_index_spgist
 test: create_view
diff --git a/src/test/regress/sql/create_schema.sql b/src/test/regress/sql/create_schema.sql
new file mode 100644 (file)
index 0000000..a367b2c
--- /dev/null
@@ -0,0 +1,70 @@
+--
+-- CREATE_SCHEMA
+--
+
+-- Schema creation with elements.
+
+CREATE ROLE regress_create_schema_role SUPERUSER;
+
+-- Cases where schema creation fails as objects are qualified with a schema
+-- that does not match with what's expected.
+-- This checks all the object types that include schema qualifications.
+CREATE SCHEMA AUTHORIZATION regress_create_schema_role
+  CREATE SEQUENCE schema_not_existing.seq;
+CREATE SCHEMA AUTHORIZATION regress_create_schema_role
+  CREATE TABLE schema_not_existing.tab (id int);
+CREATE SCHEMA AUTHORIZATION regress_create_schema_role
+  CREATE VIEW schema_not_existing.view AS SELECT 1;
+CREATE SCHEMA AUTHORIZATION regress_create_schema_role
+  CREATE INDEX ON schema_not_existing.tab (id);
+CREATE SCHEMA AUTHORIZATION regress_create_schema_role
+  CREATE TRIGGER schema_trig BEFORE INSERT ON schema_not_existing.tab
+  EXECUTE FUNCTION schema_trig.no_func();
+-- Again, with a role specification and no schema names.
+SET ROLE regress_create_schema_role;
+CREATE SCHEMA AUTHORIZATION CURRENT_USER
+  CREATE SEQUENCE schema_not_existing.seq;
+CREATE SCHEMA AUTHORIZATION CURRENT_USER
+  CREATE TABLE schema_not_existing.tab (id int);
+CREATE SCHEMA AUTHORIZATION CURRENT_USER
+  CREATE VIEW schema_not_existing.view AS SELECT 1;
+CREATE SCHEMA AUTHORIZATION CURRENT_USER
+  CREATE INDEX ON schema_not_existing.tab (id);
+CREATE SCHEMA AUTHORIZATION CURRENT_USER
+  CREATE TRIGGER schema_trig BEFORE INSERT ON schema_not_existing.tab
+  EXECUTE FUNCTION schema_trig.no_func();
+-- Again, with a schema name and a role specification.
+CREATE SCHEMA regress_schema_1 AUTHORIZATION CURRENT_USER
+  CREATE SEQUENCE schema_not_existing.seq;
+CREATE SCHEMA regress_schema_1 AUTHORIZATION CURRENT_USER
+  CREATE TABLE schema_not_existing.tab (id int);
+CREATE SCHEMA regress_schema_1 AUTHORIZATION CURRENT_USER
+  CREATE VIEW schema_not_existing.view AS SELECT 1;
+CREATE SCHEMA regress_schema_1 AUTHORIZATION CURRENT_USER
+  CREATE INDEX ON schema_not_existing.tab (id);
+CREATE SCHEMA regress_schema_1 AUTHORIZATION CURRENT_USER
+  CREATE TRIGGER schema_trig BEFORE INSERT ON schema_not_existing.tab
+  EXECUTE FUNCTION schema_trig.no_func();
+RESET ROLE;
+
+-- Cases where the schema creation succeeds.
+-- The schema created matches the role name.
+CREATE SCHEMA AUTHORIZATION regress_create_schema_role
+  CREATE TABLE regress_create_schema_role.tab (id int);
+\d regress_create_schema_role.tab
+DROP SCHEMA regress_create_schema_role CASCADE;
+-- Again, with a different role specification and no schema names.
+SET ROLE regress_create_schema_role;
+CREATE SCHEMA AUTHORIZATION CURRENT_USER
+  CREATE TABLE regress_create_schema_role.tab (id int);
+\d regress_create_schema_role.tab
+DROP SCHEMA regress_create_schema_role CASCADE;
+-- Again, with a schema name and a role specification.
+CREATE SCHEMA regress_schema_1 AUTHORIZATION CURRENT_USER
+  CREATE TABLE regress_schema_1.tab (id int);
+\d regress_schema_1.tab
+DROP SCHEMA regress_schema_1 CASCADE;
+RESET ROLE;
+
+-- Clean up
+DROP ROLE regress_create_schema_role;