Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitc2e08b0

Browse files
committed
Fix RLS policy usage in MERGE.
If MERGE executes an UPDATE action on a table with row-level security,the code incorrectly applied the WITH CHECK clauses from the targettable's INSERT policies to new rows, instead of the clauses from thetable's UPDATE policies. In addition, it failed to check new rowsagainst the target table's SELECT policies, if SELECT permissions wererequired (likely to always be the case).In addition, if MERGE executes a DO NOTHING action for matched rows,the code incorrectly applied the USING clauses from the target table'sDELETE policies to existing target tuples. These policies were appliedas checks that would throw an error, if they did not pass.Fix this, so that a MERGE UPDATE action applies the same RLS policiesas a plain UPDATE query with a WHERE clause, and a DO NOTHING actiondoes not apply any RLS checks (other than adding clauses from SELECTpolicies to the join).Back-patch to v15, where MERGE was introduced.Dean Rasheed, reviewed by Stephen Frost.Security:CVE-2023-39418
1 parenteeb4eee commitc2e08b0

File tree

4 files changed

+152
-50
lines changed

4 files changed

+152
-50
lines changed

‎src/backend/executor/nodeModifyTable.c

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2861,13 +2861,14 @@ ExecMergeMatched(ModifyTableContext *context, ResultRelInfo *resultRelInfo,
28612861
* UPDATE/DELETE RLS policies. If those checks fail, we throw an
28622862
* error.
28632863
*
2864-
* The WITH CHECK quals are applied in ExecUpdate() and hence we need
2865-
* not do anything special to handle them.
2864+
* The WITH CHECK quals for UPDATE RLS policies are applied in
2865+
* ExecUpdateAct() and hence we need not do anything special to handle
2866+
* them.
28662867
*
28672868
* NOTE: We must do this after WHEN quals are evaluated, so that we
28682869
* check policies only when they matter.
28692870
*/
2870-
if (resultRelInfo->ri_WithCheckOptions)
2871+
if (resultRelInfo->ri_WithCheckOptions&&commandType!=CMD_NOTHING)
28712872
{
28722873
ExecWithCheckOptions(commandType==CMD_UPDATE ?
28732874
WCO_RLS_MERGE_UPDATE_CHECK :WCO_RLS_MERGE_DELETE_CHECK,

‎src/backend/rewrite/rowsecurity.c

Lines changed: 60 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -394,7 +394,11 @@ get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index,
394394
* and set them up so that we can enforce the appropriate policy depending
395395
* on the final action we take.
396396
*
397-
* We already fetched the SELECT policies above.
397+
* We already fetched the SELECT policies above, to check existing rows,
398+
* but we must also check that new rows created by UPDATE actions are
399+
* visible, if SELECT rights are required for this relation. We don't do
400+
* this for INSERT actions, since an INSERT command would only do this
401+
* check if it had a RETURNING list, and MERGE does not support RETURNING.
398402
*
399403
* We don't push the UPDATE/DELETE USING quals to the RTE because we don't
400404
* really want to apply them while scanning the relation since we don't
@@ -410,40 +414,80 @@ get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index,
410414
*/
411415
if (commandType==CMD_MERGE)
412416
{
413-
List*merge_permissive_policies;
414-
List*merge_restrictive_policies;
417+
List*merge_update_permissive_policies;
418+
List*merge_update_restrictive_policies;
419+
List*merge_delete_permissive_policies;
420+
List*merge_delete_restrictive_policies;
421+
List*merge_insert_permissive_policies;
422+
List*merge_insert_restrictive_policies;
415423

416424
/*
417425
* Fetch the UPDATE policies and set them up to execute on the
418426
* existing target row before doing UPDATE.
419427
*/
420428
get_policies_for_relation(rel,CMD_UPDATE,user_id,
421-
&merge_permissive_policies,
422-
&merge_restrictive_policies);
429+
&merge_update_permissive_policies,
430+
&merge_update_restrictive_policies);
423431

424432
/*
425433
* WCO_RLS_MERGE_UPDATE_CHECK is used to check UPDATE USING quals on
426434
* the existing target row.
427435
*/
428436
add_with_check_options(rel,rt_index,
429437
WCO_RLS_MERGE_UPDATE_CHECK,
430-
merge_permissive_policies,
431-
merge_restrictive_policies,
438+
merge_update_permissive_policies,
439+
merge_update_restrictive_policies,
432440
withCheckOptions,
433441
hasSubLinks,
434442
true);
435443

444+
/* Enforce the WITH CHECK clauses of the UPDATE policies */
445+
add_with_check_options(rel,rt_index,
446+
WCO_RLS_UPDATE_CHECK,
447+
merge_update_permissive_policies,
448+
merge_update_restrictive_policies,
449+
withCheckOptions,
450+
hasSubLinks,
451+
false);
452+
453+
/*
454+
* Add ALL/SELECT policies as WCO_RLS_UPDATE_CHECK WCOs, to ensure
455+
* that the updated row is visible when executing an UPDATE action, if
456+
* SELECT rights are required for this relation.
457+
*/
458+
if (perminfo->requiredPerms&ACL_SELECT)
459+
{
460+
List*merge_select_permissive_policies;
461+
List*merge_select_restrictive_policies;
462+
463+
get_policies_for_relation(rel,CMD_SELECT,user_id,
464+
&merge_select_permissive_policies,
465+
&merge_select_restrictive_policies);
466+
add_with_check_options(rel,rt_index,
467+
WCO_RLS_UPDATE_CHECK,
468+
merge_select_permissive_policies,
469+
merge_select_restrictive_policies,
470+
withCheckOptions,
471+
hasSubLinks,
472+
true);
473+
}
474+
436475
/*
437-
* Same with DELETE policies.
476+
* Fetch the DELETE policies and set them up to execute on the
477+
* existing target row before doing DELETE.
438478
*/
439479
get_policies_for_relation(rel,CMD_DELETE,user_id,
440-
&merge_permissive_policies,
441-
&merge_restrictive_policies);
480+
&merge_delete_permissive_policies,
481+
&merge_delete_restrictive_policies);
442482

483+
/*
484+
* WCO_RLS_MERGE_DELETE_CHECK is used to check DELETE USING quals on
485+
* the existing target row.
486+
*/
443487
add_with_check_options(rel,rt_index,
444488
WCO_RLS_MERGE_DELETE_CHECK,
445-
merge_permissive_policies,
446-
merge_restrictive_policies,
489+
merge_delete_permissive_policies,
490+
merge_delete_restrictive_policies,
447491
withCheckOptions,
448492
hasSubLinks,
449493
true);
@@ -454,22 +498,13 @@ get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index,
454498
* withCheckOptions.
455499
*/
456500
get_policies_for_relation(rel,CMD_INSERT,user_id,
457-
&merge_permissive_policies,
458-
&merge_restrictive_policies);
501+
&merge_insert_permissive_policies,
502+
&merge_insert_restrictive_policies);
459503

460504
add_with_check_options(rel,rt_index,
461505
WCO_RLS_INSERT_CHECK,
462-
merge_permissive_policies,
463-
merge_restrictive_policies,
464-
withCheckOptions,
465-
hasSubLinks,
466-
false);
467-
468-
/* Enforce the WITH CHECK clauses of the UPDATE policies */
469-
add_with_check_options(rel,rt_index,
470-
WCO_RLS_UPDATE_CHECK,
471-
merge_permissive_policies,
472-
merge_restrictive_policies,
506+
merge_insert_permissive_policies,
507+
merge_insert_restrictive_policies,
473508
withCheckOptions,
474509
hasSubLinks,
475510
false);

‎src/test/regress/expected/rowsecurity.out

Lines changed: 45 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2121,10 +2121,10 @@ ALTER TABLE document ADD COLUMN dnotes text DEFAULT '';
21212121
CREATE POLICY p1 ON document FOR SELECT USING (true);
21222122
-- one may insert documents only authored by them
21232123
CREATE POLICY p2 ON document FOR INSERT WITH CHECK (dauthor = current_user);
2124-
-- one may only update documents in 'novel' category
2124+
-- one may only update documents in 'novel' category and new dlevel must be > 0
21252125
CREATE POLICY p3 ON document FOR UPDATE
21262126
USING (cid = (SELECT cid from category WHERE cname = 'novel'))
2127-
WITH CHECK (dauthor = current_user);
2127+
WITH CHECK (dlevel > 0);
21282128
-- one may only delete documents in 'manga' category
21292129
CREATE POLICY p4 ON document FOR DELETE
21302130
USING (cid = (SELECT cid from category WHERE cname = 'manga'));
@@ -2148,25 +2148,25 @@ SELECT * FROM document;
21482148
(14 rows)
21492149

21502150
SET SESSION AUTHORIZATION regress_rls_bob;
2151-
-- Fails, since update violates WITH CHECK qual ondauthor
2151+
-- Fails, since update violates WITH CHECK qual ondlevel
21522152
MERGE INTO document d
21532153
USING (SELECT 1 as sdid) s
21542154
ON did = s.sdid
21552155
WHEN MATCHED THEN
2156-
UPDATE SET dnotes = dnotes || ' notes added by merge1 ',dauthor ='regress_rls_alice';
2156+
UPDATE SET dnotes = dnotes || ' notes added by merge1 ',dlevel =0;
21572157
ERROR: new row violates row-level security policy for table "document"
21582158
-- Should be OK since USING and WITH CHECK quals pass
21592159
MERGE INTO document d
21602160
USING (SELECT 1 as sdid) s
21612161
ON did = s.sdid
21622162
WHEN MATCHED THEN
21632163
UPDATE SET dnotes = dnotes || ' notes added by merge2 ';
2164-
-- Even whendauthor is updated explicitly, but to the existing value
2164+
-- Even whendlevel is updated explicitly, but to the existing value
21652165
MERGE INTO document d
21662166
USING (SELECT 1 as sdid) s
21672167
ON did = s.sdid
21682168
WHEN MATCHED THEN
2169-
UPDATE SET dnotes = dnotes || ' notes added by merge3 ',dauthor ='regress_rls_bob';
2169+
UPDATE SET dnotes = dnotes || ' notes added by merge3 ',dlevel =1;
21702170
-- There is a MATCH for did = 3, but UPDATE's USING qual does not allow
21712171
-- updating an item in category 'science fiction'
21722172
MERGE INTO document d
@@ -2205,6 +2205,14 @@ WHEN MATCHED AND dnotes <> '' THEN
22052205
WHEN MATCHED THEN
22062206
DELETE;
22072207
ERROR: target row violates row-level security policy (USING expression) for table "document"
2208+
-- OK if DELETE is replaced with DO NOTHING
2209+
MERGE INTO document d
2210+
USING (SELECT 4 as sdid) s
2211+
ON did = s.sdid
2212+
WHEN MATCHED AND dnotes <> '' THEN
2213+
UPDATE SET dnotes = dnotes || ' notes added by merge '
2214+
WHEN MATCHED THEN
2215+
DO NOTHING;
22082216
SELECT * FROM document WHERE did = 4;
22092217
did | cid | dlevel | dauthor | dtitle | dnotes
22102218
-----+-----+--------+-----------------+----------------+--------
@@ -2253,30 +2261,53 @@ WHEN MATCHED THEN
22532261
WHEN NOT MATCHED THEN
22542262
INSERT VALUES (12, 11, 1, 'regress_rls_bob', 'another novel');
22552263
-- drop and create a new SELECT policy which prevents us from reading
2256-
-- any document except with category 'magna'
2264+
-- any document except with category 'novel'
22572265
RESET SESSION AUTHORIZATION;
22582266
DROP POLICY p1 ON document;
22592267
CREATE POLICY p1 ON document FOR SELECT
2260-
USING (cid = (SELECT cid from category WHERE cname = 'manga'));
2268+
USING (cid = (SELECT cid from category WHERE cname = 'novel'));
22612269
SET SESSION AUTHORIZATION regress_rls_bob;
22622270
-- MERGE can no longer see the matching row and hence attempts the
22632271
-- NOT MATCHED action, which results in unique key violation
22642272
MERGE INTO document d
2265-
USING (SELECT1 as sdid) s
2273+
USING (SELECT7 as sdid) s
22662274
ON did = s.sdid
22672275
WHEN MATCHED THEN
22682276
UPDATE SET dnotes = dnotes || ' notes added by merge5 '
22692277
WHEN NOT MATCHED THEN
22702278
INSERT VALUES (12, 11, 1, 'regress_rls_bob', 'another novel');
22712279
ERROR: duplicate key value violates unique constraint "document_pkey"
2280+
-- UPDATE action fails if new row is not visible
2281+
MERGE INTO document d
2282+
USING (SELECT 1 as sdid) s
2283+
ON did = s.sdid
2284+
WHEN MATCHED THEN
2285+
UPDATE SET dnotes = dnotes || ' notes added by merge6 ',
2286+
cid = (SELECT cid from category WHERE cname = 'technology');
2287+
ERROR: new row violates row-level security policy for table "document"
2288+
-- but OK if new row is visible
2289+
MERGE INTO document d
2290+
USING (SELECT 1 as sdid) s
2291+
ON did = s.sdid
2292+
WHEN MATCHED THEN
2293+
UPDATE SET dnotes = dnotes || ' notes added by merge7 ',
2294+
cid = (SELECT cid from category WHERE cname = 'novel');
2295+
-- OK to insert a new row that is not visible
2296+
MERGE INTO document d
2297+
USING (SELECT 13 as sdid) s
2298+
ON did = s.sdid
2299+
WHEN MATCHED THEN
2300+
UPDATE SET dnotes = dnotes || ' notes added by merge8 '
2301+
WHEN NOT MATCHED THEN
2302+
INSERT VALUES (13, 44, 1, 'regress_rls_bob', 'new manga');
22722303
RESET SESSION AUTHORIZATION;
22732304
-- drop the restrictive SELECT policy so that we can look at the
22742305
-- final state of the table
22752306
DROP POLICY p1 ON document;
22762307
-- Just check everything went per plan
22772308
SELECT * FROM document;
2278-
did | cid | dlevel | dauthor | dtitle | dnotes
2279-
-----+-----+--------+-------------------+----------------------------------+-----------------------------------------------------------------------
2309+
did | cid | dlevel | dauthor | dtitle |dnotes
2310+
-----+-----+--------+-------------------+----------------------------------+----------------------------------------------------------------------------------------------
22802311
3 | 22 | 2 | regress_rls_bob | my science fiction |
22812312
5 | 44 | 2 | regress_rls_bob | my second manga |
22822313
6 | 22 | 1 | regress_rls_carol | great science fiction |
@@ -2290,8 +2321,9 @@ SELECT * FROM document;
22902321
78 | 33 | 1 | regress_rls_bob | some technology novel |
22912322
79 | 33 | 1 | regress_rls_bob | technology book, can only insert |
22922323
12 | 11 | 1 | regress_rls_bob | another novel |
2293-
1 | 11 | 1 | regress_rls_bob | my first novel | notes added by merge2 notes added by merge3 notes added by merge4
2294-
(14 rows)
2324+
1 | 11 | 1 | regress_rls_bob | my first novel | notes added by merge2 notes added by merge3 notes added by merge4 notes added by merge7
2325+
13 | 44 | 1 | regress_rls_bob | new manga |
2326+
(15 rows)
22952327

22962328
--
22972329
-- ROLE/GROUP

‎src/test/regress/sql/rowsecurity.sql

Lines changed: 43 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -821,10 +821,10 @@ ALTER TABLE document ADD COLUMN dnotes text DEFAULT '';
821821
CREATE POLICY p1ON document FORSELECT USING (true);
822822
-- one may insert documents only authored by them
823823
CREATE POLICY p2ON document FOR INSERT WITHCHECK (dauthor=current_user);
824-
-- one may only update documents in 'novel' category
824+
-- one may only update documents in 'novel' category and new dlevel must be > 0
825825
CREATE POLICY p3ON document FORUPDATE
826826
USING (cid= (SELECT cidfrom categoryWHERE cname='novel'))
827-
WITHCHECK (dauthor=current_user);
827+
WITHCHECK (dlevel>0);
828828
-- one may only delete documents in 'manga' category
829829
CREATE POLICY p4ON document FORDELETE
830830
USING (cid= (SELECT cidfrom categoryWHERE cname='manga'));
@@ -833,12 +833,12 @@ SELECT * FROM document;
833833

834834
SET SESSION AUTHORIZATION regress_rls_bob;
835835

836-
-- Fails, since update violates WITH CHECK qual ondauthor
836+
-- Fails, since update violates WITH CHECK qual ondlevel
837837
MERGE INTO document d
838838
USING (SELECT1as sdid) s
839839
ON did=s.sdid
840840
WHEN MATCHED THEN
841-
UPDATESET dnotes= dnotes||' notes added by merge1',dauthor='regress_rls_alice';
841+
UPDATESET dnotes= dnotes||' notes added by merge1',dlevel=0;
842842

843843
-- Should be OK since USING and WITH CHECK quals pass
844844
MERGE INTO document d
@@ -847,12 +847,12 @@ ON did = s.sdid
847847
WHEN MATCHED THEN
848848
UPDATESET dnotes= dnotes||' notes added by merge2';
849849

850-
-- Even whendauthor is updated explicitly, but to the existing value
850+
-- Even whendlevel is updated explicitly, but to the existing value
851851
MERGE INTO document d
852852
USING (SELECT1as sdid) s
853853
ON did=s.sdid
854854
WHEN MATCHED THEN
855-
UPDATESET dnotes= dnotes||' notes added by merge3',dauthor='regress_rls_bob';
855+
UPDATESET dnotes= dnotes||' notes added by merge3',dlevel=1;
856856

857857
-- There is a MATCH for did = 3, but UPDATE's USING qual does not allow
858858
-- updating an item in category 'science fiction'
@@ -892,6 +892,15 @@ WHEN MATCHED AND dnotes <> '' THEN
892892
WHEN MATCHED THEN
893893
DELETE;
894894

895+
-- OK if DELETE is replaced with DO NOTHING
896+
MERGE INTO document d
897+
USING (SELECT4as sdid) s
898+
ON did=s.sdid
899+
WHEN MATCHEDAND dnotes<>'' THEN
900+
UPDATESET dnotes= dnotes||' notes added by merge'
901+
WHEN MATCHED THEN
902+
DO NOTHING;
903+
895904
SELECT*FROM documentWHERE did=4;
896905

897906
-- Switch to regress_rls_carol role and try the DELETE again. It should succeed
@@ -941,24 +950,49 @@ WHEN NOT MATCHED THEN
941950
INSERTVALUES (12,11,1,'regress_rls_bob','another novel');
942951

943952
-- drop and create a new SELECT policy which prevents us from reading
944-
-- any document except with category 'magna'
953+
-- any document except with category 'novel'
945954
RESET SESSION AUTHORIZATION;
946955
DROP POLICY p1ON document;
947956
CREATE POLICY p1ON document FORSELECT
948-
USING (cid= (SELECT cidfrom categoryWHERE cname='manga'));
957+
USING (cid= (SELECT cidfrom categoryWHERE cname='novel'));
949958

950959
SET SESSION AUTHORIZATION regress_rls_bob;
951960

952961
-- MERGE can no longer see the matching row and hence attempts the
953962
-- NOT MATCHED action, which results in unique key violation
954963
MERGE INTO document d
955-
USING (SELECT1as sdid) s
964+
USING (SELECT7as sdid) s
956965
ON did=s.sdid
957966
WHEN MATCHED THEN
958967
UPDATESET dnotes= dnotes||' notes added by merge5'
959968
WHEN NOT MATCHED THEN
960969
INSERTVALUES (12,11,1,'regress_rls_bob','another novel');
961970

971+
-- UPDATE action fails if new row is not visible
972+
MERGE INTO document d
973+
USING (SELECT1as sdid) s
974+
ON did=s.sdid
975+
WHEN MATCHED THEN
976+
UPDATESET dnotes= dnotes||' notes added by merge6',
977+
cid= (SELECT cidfrom categoryWHERE cname='technology');
978+
979+
-- but OK if new row is visible
980+
MERGE INTO document d
981+
USING (SELECT1as sdid) s
982+
ON did=s.sdid
983+
WHEN MATCHED THEN
984+
UPDATESET dnotes= dnotes||' notes added by merge7',
985+
cid= (SELECT cidfrom categoryWHERE cname='novel');
986+
987+
-- OK to insert a new row that is not visible
988+
MERGE INTO document d
989+
USING (SELECT13as sdid) s
990+
ON did=s.sdid
991+
WHEN MATCHED THEN
992+
UPDATESET dnotes= dnotes||' notes added by merge8'
993+
WHEN NOT MATCHED THEN
994+
INSERTVALUES (13,44,1,'regress_rls_bob','new manga');
995+
962996
RESET SESSION AUTHORIZATION;
963997
-- drop the restrictive SELECT policy so that we can look at the
964998
-- final state of the table

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp