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

Commit187e5d8

Browse files
committed
Disallow RESET ROLE and RESET SESSION AUTHORIZATION inside security-definer
functions.This extends the previous patch that forbade SETting these variables insidesecurity-definer functions. RESET is equally a security hole, since itwould allow regaining privileges of the caller; furthermore it can triggerAssert failures and perhaps other internal errors, since the code is notexpecting these variables to change in such contexts. The previous patchdid not cover this case because assign hooks don't really have enoughinformation, so move the responsibility for preventing this into guc.c.Problem discovered by Heikki Linnakangas.Security: no CVE assigned yet, extendsCVE-2007-6600
1 parentd0a368c commit187e5d8

File tree

3 files changed

+33
-38
lines changed

3 files changed

+33
-38
lines changed

‎src/backend/commands/variable.c

Lines changed: 1 addition & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
*
1010
*
1111
* IDENTIFICATION
12-
* $PostgreSQL: pgsql/src/backend/commands/variable.c,v 1.130 2009/06/11 14:48:56 momjian Exp $
12+
* $PostgreSQL: pgsql/src/backend/commands/variable.c,v 1.131 2009/09/03 22:08:05 tgl Exp $
1313
*
1414
*-------------------------------------------------------------------------
1515
*/
@@ -717,21 +717,6 @@ assign_session_authorization(const char *value, bool doit, GucSource source)
717717
/* not a saved ID, so look it up */
718718
HeapTupleroleTup;
719719

720-
if (InSecurityDefinerContext())
721-
{
722-
/*
723-
* Disallow SET SESSION AUTHORIZATION inside a security definer
724-
* context. We need to do this because when we exit the context,
725-
* GUC won't be notified, leaving things out of sync. Note that
726-
* this test is positioned so that restoring a previously saved
727-
* setting isn't prevented.
728-
*/
729-
ereport(GUC_complaint_elevel(source),
730-
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
731-
errmsg("cannot set session authorization within security-definer function")));
732-
returnNULL;
733-
}
734-
735720
if (!IsTransactionState())
736721
{
737722
/*
@@ -838,24 +823,6 @@ assign_role(const char *value, bool doit, GucSource source)
838823
}
839824
}
840825

841-
if (roleid==InvalidOid&&InSecurityDefinerContext())
842-
{
843-
/*
844-
* Disallow SET ROLE inside a security definer context. We need to do
845-
* this because when we exit the context, GUC won't be notified,
846-
* leaving things out of sync.Note that this test is arranged so
847-
* that restoring a previously saved setting isn't prevented.
848-
*
849-
* XXX it would be nice to allow this case in future, with the
850-
* behavior being that the SET ROLE's effects end when the security
851-
* definer context is exited.
852-
*/
853-
ereport(GUC_complaint_elevel(source),
854-
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
855-
errmsg("cannot set role within security-definer function")));
856-
returnNULL;
857-
}
858-
859826
if (roleid==InvalidOid&&
860827
strcmp(actual_rolename,"none")!=0)
861828
{

‎src/backend/utils/misc/guc.c

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
* Written by Peter Eisentraut <peter_e@gmx.net>.
1111
*
1212
* IDENTIFICATION
13-
* $PostgreSQL: pgsql/src/backend/utils/misc/guc.c,v 1.514 2009/08/31 19:41:00 tgl Exp $
13+
* $PostgreSQL: pgsql/src/backend/utils/misc/guc.c,v 1.515 2009/09/03 22:08:05 tgl Exp $
1414
*
1515
*--------------------------------------------------------------------
1616
*/
@@ -2321,7 +2321,7 @@ static struct config_string ConfigureNamesString[] =
23212321
{"role",PGC_USERSET,UNGROUPED,
23222322
gettext_noop("Sets the current role."),
23232323
NULL,
2324-
GUC_IS_NAME |GUC_NO_SHOW_ALL |GUC_NO_RESET_ALL |GUC_NOT_IN_SAMPLE |GUC_DISALLOW_IN_FILE
2324+
GUC_IS_NAME |GUC_NO_SHOW_ALL |GUC_NO_RESET_ALL |GUC_NOT_IN_SAMPLE |GUC_DISALLOW_IN_FILE |GUC_NOT_WHILE_SEC_DEF
23252325
},
23262326
&role_string,
23272327
"none",assign_role,show_role
@@ -2332,7 +2332,7 @@ static struct config_string ConfigureNamesString[] =
23322332
{"session_authorization",PGC_USERSET,UNGROUPED,
23332333
gettext_noop("Sets the session user name."),
23342334
NULL,
2335-
GUC_IS_NAME |GUC_REPORT |GUC_NO_SHOW_ALL |GUC_NO_RESET_ALL |GUC_NOT_IN_SAMPLE |GUC_DISALLOW_IN_FILE
2335+
GUC_IS_NAME |GUC_REPORT |GUC_NO_SHOW_ALL |GUC_NO_RESET_ALL |GUC_NOT_IN_SAMPLE |GUC_DISALLOW_IN_FILE |GUC_NOT_WHILE_SEC_DEF
23362336
},
23372337
&session_authorization_string,
23382338
NULL,assign_session_authorization,show_session_authorization
@@ -4661,6 +4661,32 @@ set_config_option(const char *name, const char *value,
46614661
break;
46624662
}
46634663

4664+
/*
4665+
* Disallow changing GUC_NOT_WHILE_SEC_DEF values if we are inside a
4666+
* security-definer function. We can reject this regardless of
4667+
* the context or source, mainly because sources that it might be
4668+
* reasonable to override for won't be seen while inside a function.
4669+
*
4670+
* Note: variables marked GUC_NOT_WHILE_SEC_DEF should probably be marked
4671+
* GUC_NO_RESET_ALL as well, because ResetAllOptions() doesn't check this.
4672+
*
4673+
* Note: this flag is currently used for "session_authorization" and
4674+
* "role". We need to prohibit this because when we exit the sec-def
4675+
* context, GUC won't be notified, leaving things out of sync.
4676+
*
4677+
* XXX it would be nice to allow these cases in future, with the behavior
4678+
* being that the SET's effects end when the security definer context is
4679+
* exited.
4680+
*/
4681+
if ((record->flags&GUC_NOT_WHILE_SEC_DEF)&&InSecurityDefinerContext())
4682+
{
4683+
ereport(elevel,
4684+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
4685+
errmsg("cannot set parameter \"%s\" within security-definer function",
4686+
name)));
4687+
return false;
4688+
}
4689+
46644690
/*
46654691
* Should we set reset/stacked values?(If so, the behavior is not
46664692
* transactional.)This is done either when we get a default value from

‎src/include/utils/guc.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* Copyright (c) 2000-2009, PostgreSQL Global Development Group
88
* Written by Peter Eisentraut <peter_e@gmx.net>.
99
*
10-
* $PostgreSQL: pgsql/src/include/utils/guc.h,v 1.103 2009/08/29 19:26:52 tgl Exp $
10+
* $PostgreSQL: pgsql/src/include/utils/guc.h,v 1.104 2009/09/03 22:08:05 tgl Exp $
1111
*--------------------------------------------------------------------
1212
*/
1313
#ifndefGUC_H
@@ -146,6 +146,8 @@ typedef enum
146146
#defineGUC_UNIT_MIN0x4000/* value is in minutes */
147147
#defineGUC_UNIT_TIME0x7000/* mask for MS, S, MIN */
148148

149+
#defineGUC_NOT_WHILE_SEC_DEF0x8000/* can't change inside sec-def func */
150+
149151
/* GUC vars that are actually declared in guc.c, rather than elsewhere */
150152
externboollog_duration;
151153
externboolDebug_print_plan;

0 commit comments

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