Prohibit transaction commands in security definer procedures

Starting and aborting transactions in security definer procedures
doesn't work.  StartTransaction() insists that the security context
stack is empty, so this would currently cause a crash, and
AbortTransaction() resets it.  This could be made to work by
reorganizing the code, but right now we just prohibit it.

Reported-by: amul sul <sulamul@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/CAAJ_b96Gupt_LFL7uNyy3c50-wbhA68NUjiK5%3DrF6_w%3Dpq_T%3DQ%40mail.gmail.com

diff --git a/doc/src/sgml/ref/create_procedure.sgml b/doc/src/sgml/ref/create_procedure.sgml
index f3c3bb006cf52a50c74cc86513741c69acd96821..6c1de34b01231fb8a18e3992192afeef53cdce34 100644 (file)
--- a/doc/src/sgml/ref/create_procedure.sgml
+++ b/doc/src/sgml/ref/create_procedure.sgml
@@ -203,6 +203,12 @@ CREATE [ OR REPLACE ] PROCEDURE
       conformance, but it is optional since, unlike in SQL, this feature
       applies to all procedures not only external ones.
      </para>
+
+     <para>
+      A <literal>SECURITY DEFINER</literal> procedure cannot execute
+      transaction control statements (for example, <command>COMMIT</command>
+      and <command>ROLLBACK</command>, depending on the language).
+     </para>
     </listitem>
    </varlistentry>
 

diff --git a/src/backend/commands/functioncmds.c b/src/backend/commands/functioncmds.c
index 84daa19e06417aaf85cba8e59d9529bb22f9db15..68109bfda063200d8724b6e5723052e0aa296c1b 100644 (file)
--- a/src/backend/commands/functioncmds.c
+++ b/src/backend/commands/functioncmds.c
@@ -2245,6 +2245,15 @@ ExecuteCallStmt(CallStmt *stmt, ParamListInfo params, bool atomic, DestReceiver
    if (!heap_attisnull(tp, Anum_pg_proc_proconfig, NULL))
        callcontext->atomic = true;
 
+   /*
+    * In security definer procedures, we can't allow transaction commands.
+    * StartTransaction() insists that the security context stack is empty,
+    * and AbortTransaction() resets the security context.  This could be
+    * reorganized, but right now it doesn't work.
+    */
+   if (((Form_pg_proc )GETSTRUCT(tp))->prosecdef)
+       callcontext->atomic = true;
+
    /*
     * Expand named arguments, defaults, etc.
     */

diff --git a/src/pl/plpgsql/src/expected/plpgsql_transaction.out b/src/pl/plpgsql/src/expected/plpgsql_transaction.out
index 274b2c6f1709d90bd643fcfe9ef72266812bbcce..0b5a039b89cf3e63b55a71c5c1c2e299cbcf6983 100644 (file)
--- a/src/pl/plpgsql/src/expected/plpgsql_transaction.out
+++ b/src/pl/plpgsql/src/expected/plpgsql_transaction.out
@@ -130,6 +130,18 @@ $$;
 CALL transaction_test5();
 ERROR:  invalid transaction termination
 CONTEXT:  PL/pgSQL function transaction_test5() line 3 at COMMIT
+-- SECURITY DEFINER currently disallow transaction statements
+CREATE PROCEDURE transaction_test5b()
+LANGUAGE plpgsql
+SECURITY DEFINER
+AS $$
+BEGIN
+    COMMIT;
+END;
+$$;
+CALL transaction_test5b();
+ERROR:  invalid transaction termination
+CONTEXT:  PL/pgSQL function transaction_test5b() line 3 at COMMIT
 TRUNCATE test1;
 -- nested procedure calls
 CREATE PROCEDURE transaction_test6(c text)

diff --git a/src/pl/plpgsql/src/sql/plpgsql_transaction.sql b/src/pl/plpgsql/src/sql/plpgsql_transaction.sql
index 1624aed6eca27578f8ab0f0baa7746511d553878..236db9bf2bfd4c6c2f900d5b59cf738cc234d46e 100644 (file)
--- a/src/pl/plpgsql/src/sql/plpgsql_transaction.sql
+++ b/src/pl/plpgsql/src/sql/plpgsql_transaction.sql
@@ -116,6 +116,19 @@ $$;
 CALL transaction_test5();
 
 
+-- SECURITY DEFINER currently disallow transaction statements
+CREATE PROCEDURE transaction_test5b()
+LANGUAGE plpgsql
+SECURITY DEFINER
+AS $$
+BEGIN
+    COMMIT;
+END;
+$$;
+
+CALL transaction_test5b();
+
+
 TRUNCATE test1;
 
 -- nested procedure calls