database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-07-28 23:42:10 +03:00
Code Activity

Disallow RESET ROLE and RESET SESSION AUTHORIZATION inside security-definer

Browse Source 
functions.

This extends the previous patch that forbade SETting these variables inside
security-definer functions.  RESET is equally a security hole, since it
would allow regaining privileges of the caller; furthermore it can trigger
Assert failures and perhaps other internal errors, since the code is not
expecting these variables to change in such contexts.  The previous patch
did not cover this case because assign hooks don't really have enough
information, so move the responsibility for preventing this into guc.c.

Problem discovered by Heikki Linnakangas.

Security: no CVE assigned yet, extends CVE-2007-6600
This commit is contained in:
Tom Lane
2009-09-03 22:08:45 +00:00
parent 54e070ba08
commit 0ed2edfaae
3 changed files with 33 additions and 40 deletions
Download Patch File Download Diff File Expand all files Collapse all files

32
src/backend/utils/misc/guc.c
View File

@ -10,7 +10,7 @@
* Written by Peter Eisentraut <peter_e@gmx.net>.
*
* IDENTIFICATION
* $PostgreSQL: pgsql/src/backend/utils/misc/guc.c,v 1.299.2.4 2008/05/26 18:54:50 tgl Exp $
* $PostgreSQL: pgsql/src/backend/utils/misc/guc.c,v 1.299.2.5 2009/09/03 22:08:45 tgl Exp $
*
*--------------------------------------------------------------------
*/
@ -1936,7 +1936,7 @@ static struct config_string ConfigureNamesString[] =
{"role", PGC_USERSET, UNGROUPED,
gettext_noop("Sets the current role."),
NULL,
GUC_IS_NAME | GUC_NO_SHOW_ALL | GUC_NO_RESET_ALL | GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE
GUC_IS_NAME | GUC_NO_SHOW_ALL | GUC_NO_RESET_ALL | GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE | GUC_NOT_WHILE_SEC_DEF
},
&role_string,
"none", assign_role, show_role
@ -1947,7 +1947,7 @@ static struct config_string ConfigureNamesString[] =
{"session_authorization", PGC_USERSET, UNGROUPED,
gettext_noop("Sets the session user name."),
NULL,
GUC_IS_NAME | GUC_REPORT | GUC_NO_SHOW_ALL | GUC_NO_RESET_ALL | GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE
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
},
&session_authorization_string,
NULL, assign_session_authorization, show_session_authorization
@ -3665,6 +3665,32 @@ set_config_option(const char *name, const char *value,
break;
}
/*
* Disallow changing GUC_NOT_WHILE_SEC_DEF values if we are inside a
* security-definer function. We can reject this regardless of
* the context or source, mainly because sources that it might be
* reasonable to override for won't be seen while inside a function.
*
* Note: variables marked GUC_NOT_WHILE_SEC_DEF should probably be marked
* GUC_NO_RESET_ALL as well, because ResetAllOptions() doesn't check this.
*
* Note: this flag is currently used for "session_authorization" and
* "role". We need to prohibit this because when we exit the sec-def
* context, GUC won't be notified, leaving things out of sync.
*
* XXX it would be nice to allow these cases in future, with the behavior
* being that the SET's effects end when the security definer context is
* exited.
*/
if ((record->flags & GUC_NOT_WHILE_SEC_DEF) && InSecurityDefinerContext())
{
ereport(elevel,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("cannot set parameter \"%s\" within security-definer function",
name)));
return false;
}
/*
* Should we set reset/stacked values? (If so, the behavior is not
* transactional.)