diff --git a/regress/expected/security.out b/regress/expected/security.out index 59e58cb05..eaa3c956e 100644 --- a/regress/expected/security.out +++ b/regress/expected/security.out @@ -1655,3 +1655,61 @@ NOTICE: graph "rls_graph" has been dropped (1 row) +-- ============================================================================ +-- NON-SUPERUSER drop_label REGRESSION TEST +-- +-- Regression test for object_ownercheck() argument order in +-- range_var_callback_for_remove_relation(). A non-superuser that OWNS a +-- graph/label previously failed drop_label() with "unrecognized class ID" +-- because rel_oid was passed as the classid instead of RelationRelationId. +-- ============================================================================ +DROP ROLE IF EXISTS age_nonsuper; +NOTICE: role "age_nonsuper" does not exist, skipping +CREATE ROLE age_nonsuper LOGIN NOSUPERUSER; +-- create_graph() creates a new schema in the current database, so the role +-- needs CREATE on the database; managing labels needs USAGE + CREATE on +-- ag_catalog. Grant CREATE on whatever database the tests run in. +SELECT format('GRANT CREATE ON DATABASE %I TO age_nonsuper', current_database()) +\gexec +GRANT CREATE ON DATABASE contrib_regression TO age_nonsuper +GRANT USAGE ON SCHEMA ag_catalog TO age_nonsuper; +GRANT CREATE ON SCHEMA ag_catalog TO age_nonsuper; +-- Reproduce as the non-superuser role: it creates (and therefore OWNS) the +-- graph and the label, then drops the label. Before the fix this raised +-- "unrecognized class ID"; after the fix the label is dropped successfully. +SET ROLE age_nonsuper; +SELECT create_graph('repro_graph'); +NOTICE: graph "repro_graph" has been created + create_graph +-------------- + +(1 row) + +SELECT create_vlabel('repro_graph', 'repro_label'); +NOTICE: VLabel "repro_label" has been created + create_vlabel +--------------- + +(1 row) + +SELECT drop_label('repro_graph', 'repro_label'); +NOTICE: label "repro_graph"."repro_label" has been dropped + drop_label +------------ + +(1 row) + +SELECT drop_graph('repro_graph', true); +NOTICE: drop cascades to 2 other objects +DETAIL: drop cascades to table repro_graph._ag_label_vertex +drop cascades to table repro_graph._ag_label_edge +NOTICE: graph "repro_graph" has been dropped + drop_graph +------------ + +(1 row) + +RESET ROLE; +-- Cleanup +DROP OWNED BY age_nonsuper CASCADE; +DROP ROLE age_nonsuper; diff --git a/regress/sql/security.sql b/regress/sql/security.sql index 344dd23d4..af5b3ccb6 100644 --- a/regress/sql/security.sql +++ b/regress/sql/security.sql @@ -1449,3 +1449,37 @@ DROP ROLE rls_admin; -- Drop test graph SELECT drop_graph('rls_graph', true); + +-- ============================================================================ +-- NON-SUPERUSER drop_label REGRESSION TEST +-- +-- Regression test for object_ownercheck() argument order in +-- range_var_callback_for_remove_relation(). A non-superuser that OWNS a +-- graph/label previously failed drop_label() with "unrecognized class ID" +-- because rel_oid was passed as the classid instead of RelationRelationId. +-- ============================================================================ + +DROP ROLE IF EXISTS age_nonsuper; +CREATE ROLE age_nonsuper LOGIN NOSUPERUSER; + +-- create_graph() creates a new schema in the current database, so the role +-- needs CREATE on the database; managing labels needs USAGE + CREATE on +-- ag_catalog. Grant CREATE on whatever database the tests run in. +SELECT format('GRANT CREATE ON DATABASE %I TO age_nonsuper', current_database()) +\gexec +GRANT USAGE ON SCHEMA ag_catalog TO age_nonsuper; +GRANT CREATE ON SCHEMA ag_catalog TO age_nonsuper; + +-- Reproduce as the non-superuser role: it creates (and therefore OWNS) the +-- graph and the label, then drops the label. Before the fix this raised +-- "unrecognized class ID"; after the fix the label is dropped successfully. +SET ROLE age_nonsuper; +SELECT create_graph('repro_graph'); +SELECT create_vlabel('repro_graph', 'repro_label'); +SELECT drop_label('repro_graph', 'repro_label'); +SELECT drop_graph('repro_graph', true); +RESET ROLE; + +-- Cleanup +DROP OWNED BY age_nonsuper CASCADE; +DROP ROLE age_nonsuper; diff --git a/src/backend/commands/label_commands.c b/src/backend/commands/label_commands.c index ac789ecce..bc8d63dab 100644 --- a/src/backend/commands/label_commands.c +++ b/src/backend/commands/label_commands.c @@ -1036,7 +1036,7 @@ static void range_var_callback_for_remove_relation(const RangeVar *rel, /* relkind == expected_relkind */ - if (!object_ownercheck(rel_oid, get_rel_namespace(rel_oid), GetUserId())) + if (!object_ownercheck(RelationRelationId, rel_oid, GetUserId())) { aclcheck_error(ACLCHECK_NOT_OWNER, get_relkind_objtype(get_rel_relkind(rel_oid)),