From fb97d2b6bf03929ea70293fee37539e132ec8519 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Tue, 5 Jun 2001 19:34:56 +0000
Subject: [PATCH] Correct permissions-checking bugs associated with ancient
decision to copy PUBLIC access rights into each newly created ACL entry.
Instead treat each ACL entry as independent flags. Also clean up some
ugliness in acl.h API.
---
src/backend/catalog/aclchk.c | 186 ++++++++++++++++++++++++---------
src/backend/commands/comment.c | 24 +----
src/backend/commands/remove.c | 6 +-
src/backend/utils/adt/acl.c | 35 +++----
src/include/utils/acl.h | 16 +--
5 files changed, 162 insertions(+), 105 deletions(-)
diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c
index 5ef74cb1d55..f772ea3a534 100644
--- a/src/backend/catalog/aclchk.c
+++ b/src/backend/catalog/aclchk.c
@@ -8,7 +8,7 @@
*
*
* IDENTIFICATION
- * $Header: /cvsroot/pgsql/src/backend/catalog/aclchk.c,v 1.48 2001/05/27 09:59:28 petere Exp $
+ * $Header: /cvsroot/pgsql/src/backend/catalog/aclchk.c,v 1.49 2001/06/05 19:34:56 tgl Exp $
*
* NOTES
* See acl.h.
@@ -33,8 +33,7 @@
#include "utils/acl.h"
#include "utils/syscache.h"
-static int32 aclcheck(char *relname, Acl *acl, AclId id,
- AclIdType idtype, AclMode mode);
+static int32 aclcheck(Acl *acl, AclId id, AclIdType idtype, AclMode mode);
/* warning messages, now more explicit. */
/* MUST correspond to the order of the ACLCHK_* result codes in acl.h. */
@@ -192,6 +191,9 @@ get_groname(AclId grosysid)
return name;
}
+/*
+ * Is user a member of group?
+ */
static bool
in_group(AclId uid, AclId gid)
{
@@ -199,7 +201,7 @@ in_group(AclId uid, AclId gid)
HeapTuple tuple;
Datum att;
bool isNull;
- IdList *tmp;
+ IdList *glist;
AclId *aidp;
int i,
num;
@@ -216,10 +218,10 @@ in_group(AclId uid, AclId gid)
if (!isNull)
{
/* be sure the IdList is not toasted */
- tmp = DatumGetIdListP(att);
+ glist = DatumGetIdListP(att);
/* scan it */
- num = IDLIST_NUM(tmp);
- aidp = IDLIST_DAT(tmp);
+ num = IDLIST_NUM(glist);
+ aidp = IDLIST_DAT(glist);
for (i = 0; i < num; ++i)
{
if (aidp[i] == uid)
@@ -228,6 +230,9 @@ in_group(AclId uid, AclId gid)
break;
}
}
+ /* if IdList was toasted, free detoasted copy */
+ if ((Pointer) glist != DatumGetPointer(att))
+ pfree(glist);
}
ReleaseSysCache(tuple);
}
@@ -238,11 +243,15 @@ in_group(AclId uid, AclId gid)
/*
* aclcheck
- * Returns 1 if the 'id' of type 'idtype' has ACL entries in 'acl' to satisfy
- * any one of the requirements of 'mode'. Returns 0 otherwise.
+ *
+ * Returns ACLCHECK_OK if the 'id' of type 'idtype' has ACL entries in 'acl'
+ * to satisfy any one of the requirements of 'mode'. Returns an appropriate
+ * ACLCHECK_* error code otherwise.
+ *
+ * The ACL list is expected to be sorted in standard order.
*/
static int32
-aclcheck(char *relname, Acl *acl, AclId id, AclIdType idtype, AclMode mode)
+aclcheck(Acl *acl, AclId id, AclIdType idtype, AclMode mode)
{
AclItem *aip,
*aidat;
@@ -255,7 +264,7 @@ aclcheck(char *relname, Acl *acl, AclId id, AclIdType idtype, AclMode mode)
*/
if (!acl)
{
- elog(DEBUG, "aclcheck: null ACL, returning 1");
+ elog(DEBUG, "aclcheck: null ACL, returning OK");
return ACLCHECK_OK;
}
@@ -270,15 +279,28 @@ aclcheck(char *relname, Acl *acl, AclId id, AclIdType idtype, AclMode mode)
*/
if (num < 1)
{
- elog(DEBUG, "aclcheck: zero-length ACL, returning 1");
+ elog(DEBUG, "aclcheck: zero-length ACL, returning OK");
+ return ACLCHECK_OK;
+ }
+
+ /*
+ * "World" rights are applicable regardless of the passed-in ID,
+ * and since they're much the cheapest to check, check 'em first.
+ */
+ if (aidat->ai_idtype != ACL_IDTYPE_WORLD)
+ elog(ERROR, "aclcheck: first entry in ACL is not 'world' entry");
+ if (aidat->ai_mode & mode)
+ {
+#ifdef ACLDEBUG
+ elog(DEBUG, "aclcheck: using world=%d", aidat->ai_mode);
+#endif
return ACLCHECK_OK;
}
- Assert(aidat->ai_idtype == ACL_IDTYPE_WORLD);
switch (idtype)
{
case ACL_IDTYPE_UID:
- /* Look for exact match to user */
+ /* See if permission is granted directly to user */
for (i = 1, aip = aidat + 1; /* skip world entry */
i < num && aip->ai_idtype == ACL_IDTYPE_UID;
++i, ++aip)
@@ -289,7 +311,8 @@ aclcheck(char *relname, Acl *acl, AclId id, AclIdType idtype, AclMode mode)
elog(DEBUG, "aclcheck: found user %u/%d",
aip->ai_id, aip->ai_mode);
#endif
- return (aip->ai_mode & mode) ? ACLCHECK_OK : ACLCHECK_NO_PRIV;
+ if (aip->ai_mode & mode)
+ return ACLCHECK_OK;
}
}
/* See if he has the permission via any group */
@@ -309,15 +332,13 @@ aclcheck(char *relname, Acl *acl, AclId id, AclIdType idtype, AclMode mode)
}
}
}
- /* Else, look to the world entry */
break;
case ACL_IDTYPE_GID:
/* Look for this group ID */
- for (i = 1, aip = aidat + 1; /* skip world entry and
- * UIDs */
+ for (i = 1, aip = aidat + 1; /* skip world entry */
i < num && aip->ai_idtype == ACL_IDTYPE_UID;
++i, ++aip)
- ;
+ /* skip UID entry */;
for (;
i < num && aip->ai_idtype == ACL_IDTYPE_GID;
++i, ++aip)
@@ -328,10 +349,10 @@ aclcheck(char *relname, Acl *acl, AclId id, AclIdType idtype, AclMode mode)
elog(DEBUG, "aclcheck: found group %u/%d",
aip->ai_id, aip->ai_mode);
#endif
- return (aip->ai_mode & mode) ? ACLCHECK_OK : ACLCHECK_NO_PRIV;
+ if (aip->ai_mode & mode)
+ return ACLCHECK_OK;
}
}
- /* Else, look to the world entry */
break;
case ACL_IDTYPE_WORLD:
/* Only check the world entry */
@@ -341,12 +362,15 @@ aclcheck(char *relname, Acl *acl, AclId id, AclIdType idtype, AclMode mode)
break;
}
-#ifdef ACLDEBUG
- elog(DEBUG, "aclcheck: using world=%d", aidat->ai_mode);
-#endif
- return (aidat->ai_mode & mode) ? ACLCHECK_OK : ACLCHECK_NO_PRIV;
+ /* If get here, he doesn't have the privilege nohow */
+ return ACLCHECK_NO_PRIV;
}
+/*
+ * Exported routine for checking a user's access privileges to a table
+ *
+ * Returns an ACLCHECK_* result code.
+ */
int32
pg_aclcheck(char *relname, Oid userid, AclMode mode)
{
@@ -357,6 +381,9 @@ pg_aclcheck(char *relname, Oid userid, AclMode mode)
bool isNull;
Acl *acl;
+ /*
+ * Validate userid, find out if he is superuser
+ */
tuple = SearchSysCache(SHADOWSYSID,
ObjectIdGetDatum(userid),
0, 0, 0);
@@ -371,13 +398,15 @@ pg_aclcheck(char *relname, Oid userid, AclMode mode)
* pg_shadow.usecatupd is set. (This is to let superusers protect
* themselves from themselves.)
*/
- if (((mode & ACL_UPDATE) || (mode & ACL_INSERT) || (mode & ACL_DELETE)) &&
+ if ((mode & (ACL_INSERT | ACL_UPDATE | ACL_DELETE)) &&
!allowSystemTableMods && IsSystemRelationName(relname) &&
strncmp(relname, "pg_temp.", strlen("pg_temp.")) != 0 &&
!((Form_pg_shadow) GETSTRUCT(tuple))->usecatupd)
{
+#ifdef ACLDEBUG
elog(DEBUG, "pg_aclcheck: catalog update to \"%s\": permission denied",
relname);
+#endif
ReleaseSysCache(tuple);
return ACLCHECK_NO_PRIV;
}
@@ -416,25 +445,35 @@ pg_aclcheck(char *relname, Oid userid, AclMode mode)
ownerId = ((Form_pg_class) GETSTRUCT(tuple))->relowner;
acl = acldefault(relname, ownerId);
+ aclDatum = (Datum) 0;
}
else
{
- /* get a detoasted copy of the rel's ACL */
- acl = DatumGetAclPCopy(aclDatum);
+ /* detoast rel's ACL if necessary */
+ acl = DatumGetAclP(aclDatum);
}
- result = aclcheck(relname, acl, userid, (AclIdType) ACL_IDTYPE_UID, mode);
+ result = aclcheck(acl, userid, (AclIdType) ACL_IDTYPE_UID, mode);
- if (acl)
+ /* if we have a detoasted copy, free it */
+ if (acl && (Pointer) acl != DatumGetPointer(aclDatum))
pfree(acl);
+
ReleaseSysCache(tuple);
return result;
}
-int32
+/*
+ * Check ownership of an object identified by name (which will be looked
+ * up in the system cache identified by cacheid).
+ *
+ * Returns true if userid owns the item, or should be allowed to modify
+ * the item as if he owned it.
+ */
+bool
pg_ownercheck(Oid userid,
- const char *value,
+ const char *name,
int cacheid)
{
HeapTuple tuple;
@@ -459,39 +498,27 @@ pg_ownercheck(Oid userid,
usename);
#endif
ReleaseSysCache(tuple);
- return 1;
+ return true;
}
ReleaseSysCache(tuple);
/* caution: usename is inaccessible beyond this point... */
tuple = SearchSysCache(cacheid,
- PointerGetDatum(value),
+ PointerGetDatum(name),
0, 0, 0);
switch (cacheid)
{
- case OPEROID:
- if (!HeapTupleIsValid(tuple))
- elog(ERROR, "pg_ownercheck: operator %ld not found",
- PointerGetDatum(value));
- owner_id = ((Form_pg_operator) GETSTRUCT(tuple))->oprowner;
- break;
- case PROCNAME:
- if (!HeapTupleIsValid(tuple))
- elog(ERROR, "pg_ownercheck: function \"%s\" not found",
- value);
- owner_id = ((Form_pg_proc) GETSTRUCT(tuple))->proowner;
- break;
case RELNAME:
if (!HeapTupleIsValid(tuple))
elog(ERROR, "pg_ownercheck: class \"%s\" not found",
- value);
+ name);
owner_id = ((Form_pg_class) GETSTRUCT(tuple))->relowner;
break;
case TYPENAME:
if (!HeapTupleIsValid(tuple))
elog(ERROR, "pg_ownercheck: type \"%s\" not found",
- value);
+ name);
owner_id = ((Form_pg_type) GETSTRUCT(tuple))->typowner;
break;
default:
@@ -505,7 +532,58 @@ pg_ownercheck(Oid userid,
return userid == owner_id;
}
-int32
+/*
+ * Ownership check for an operator (specified by OID).
+ */
+bool
+pg_oper_ownercheck(Oid userid, Oid oprid)
+{
+ HeapTuple tuple;
+ AclId owner_id;
+ char *usename;
+
+ tuple = SearchSysCache(SHADOWSYSID,
+ ObjectIdGetDatum(userid),
+ 0, 0, 0);
+ if (!HeapTupleIsValid(tuple))
+ elog(ERROR, "pg_oper_ownercheck: invalid user id %u",
+ (unsigned) userid);
+ usename = NameStr(((Form_pg_shadow) GETSTRUCT(tuple))->usename);
+
+ /*
+ * Superusers bypass all permission-checking.
+ */
+ if (((Form_pg_shadow) GETSTRUCT(tuple))->usesuper)
+ {
+#ifdef ACLDEBUG
+ elog(DEBUG, "pg_ownercheck: user \"%s\" is superuser",
+ usename);
+#endif
+ ReleaseSysCache(tuple);
+ return true;
+ }
+
+ ReleaseSysCache(tuple);
+ /* caution: usename is inaccessible beyond this point... */
+
+ tuple = SearchSysCache(OPEROID,
+ ObjectIdGetDatum(oprid),
+ 0, 0, 0);
+ if (!HeapTupleIsValid(tuple))
+ elog(ERROR, "pg_ownercheck: operator %u not found",
+ oprid);
+
+ owner_id = ((Form_pg_operator) GETSTRUCT(tuple))->oprowner;
+
+ ReleaseSysCache(tuple);
+
+ return userid == owner_id;
+}
+
+/*
+ * Ownership check for a function (specified by name and argument types).
+ */
+bool
pg_func_ownercheck(Oid userid,
char *funcname,
int nargs,
@@ -533,7 +611,7 @@ pg_func_ownercheck(Oid userid,
usename);
#endif
ReleaseSysCache(tuple);
- return 1;
+ return true;
}
ReleaseSysCache(tuple);
@@ -554,7 +632,11 @@ pg_func_ownercheck(Oid userid,
return userid == owner_id;
}
-int32
+/*
+ * Ownership check for an aggregate function (specified by name and
+ * argument type).
+ */
+bool
pg_aggr_ownercheck(Oid userid,
char *aggname,
Oid basetypeID)
@@ -581,7 +663,7 @@ pg_aggr_ownercheck(Oid userid,
usename);
#endif
ReleaseSysCache(tuple);
- return 1;
+ return true;
}
ReleaseSysCache(tuple);
diff --git a/src/backend/commands/comment.c b/src/backend/commands/comment.c
index 7d3ba9b5618..695c7401e71 100644
--- a/src/backend/commands/comment.c
+++ b/src/backend/commands/comment.c
@@ -7,7 +7,7 @@
* Copyright (c) 1999, PostgreSQL Global Development Group
*
* IDENTIFICATION
- * $Header: /cvsroot/pgsql/src/backend/commands/comment.c,v 1.28 2001/05/27 09:59:29 petere Exp $
+ * $Header: /cvsroot/pgsql/src/backend/commands/comment.c,v 1.29 2001/06/05 19:34:56 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@@ -507,13 +507,9 @@ CommentType(char *type, char *comment)
/*** First, validate user ***/
-#ifndef NO_SECURITY
if (!pg_ownercheck(GetUserId(), type, TYPENAME))
- {
elog(ERROR, "you are not permitted to comment on type '%s'",
type);
- }
-#endif
/*** Next, find the type's oid ***/
@@ -561,21 +557,15 @@ CommentAggregate(char *aggregate, List *arguments, char *comment)
/*** Next, validate the user's attempt to comment ***/
-#ifndef NO_SECURITY
if (!pg_aggr_ownercheck(GetUserId(), aggregate, baseoid))
{
if (aggtypename)
- {
elog(ERROR, "you are not permitted to comment on aggregate '%s' %s '%s'",
aggregate, "with type", aggtypename);
- }
else
- {
elog(ERROR, "you are not permitted to comment on aggregate '%s'",
aggregate);
- }
}
-#endif
/*** Now, attempt to find the actual tuple in pg_aggregate ***/
@@ -646,11 +636,9 @@ CommentProc(char *function, List *arguments, char *comment)
/*** Now, validate the user's ability to comment on this function ***/
-#ifndef NO_SECURITY
if (!pg_func_ownercheck(GetUserId(), function, argcount, argoids))
elog(ERROR, "you are not permitted to comment on function '%s'",
function);
-#endif
/*** Now, find the corresponding oid for this procedure ***/
@@ -745,13 +733,9 @@ CommentOperator(char *opername, List *arguments, char *comment)
/*** Valid user's ability to comment on this operator ***/
-#ifndef NO_SECURITY
- if (!pg_ownercheck(GetUserId(), (char *) ObjectIdGetDatum(oid), OPEROID))
- {
+ if (!pg_oper_ownercheck(GetUserId(), oid))
elog(ERROR, "you are not permitted to comment on operator '%s'",
opername);
- }
-#endif
/*** Get the procedure associated with the operator ***/
@@ -792,13 +776,9 @@ CommentTrigger(char *trigger, char *relname, char *comment)
/*** First, validate the user's action ***/
-#ifndef NO_SECURITY
if (!pg_ownercheck(GetUserId(), relname, RELNAME))
- {
elog(ERROR, "you are not permitted to comment on trigger '%s' %s '%s'",
trigger, "defined for relation", relname);
- }
-#endif
/*** Now, fetch the trigger oid from pg_trigger ***/
diff --git a/src/backend/commands/remove.c b/src/backend/commands/remove.c
index da5ad74d8ba..48701a893a8 100644
--- a/src/backend/commands/remove.c
+++ b/src/backend/commands/remove.c
@@ -8,7 +8,7 @@
*
*
* IDENTIFICATION
- * $Header: /cvsroot/pgsql/src/backend/commands/Attic/remove.c,v 1.60 2001/03/22 03:59:23 momjian Exp $
+ * $Header: /cvsroot/pgsql/src/backend/commands/Attic/remove.c,v 1.61 2001/06/05 19:34:56 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@@ -81,9 +81,7 @@ RemoveOperator(char *operatorName, /* operator name */
if (HeapTupleIsValid(tup))
{
- if (!pg_ownercheck(GetUserId(),
- (char *) ObjectIdGetDatum(tup->t_data->t_oid),
- OPEROID))
+ if (!pg_oper_ownercheck(GetUserId(), tup->t_data->t_oid))
elog(ERROR, "RemoveOperator: operator '%s': permission denied",
operatorName);
diff --git a/src/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.c
index f4e3fe9986d..1da525bd032 100644
--- a/src/backend/utils/adt/acl.c
+++ b/src/backend/utils/adt/acl.c
@@ -8,14 +8,14 @@
*
*
* IDENTIFICATION
- * $Header: /cvsroot/pgsql/src/backend/utils/adt/acl.c,v 1.59 2001/05/27 09:59:30 petere Exp $
+ * $Header: /cvsroot/pgsql/src/backend/utils/adt/acl.c,v 1.60 2001/06/05 19:34:56 tgl Exp $
*
*-------------------------------------------------------------------------
*/
-#include <ctype.h>
-
#include "postgres.h"
+#include <ctype.h>
+
#include "access/heapam.h"
#include "catalog/catalog.h"
#include "catalog/pg_shadow.h"
@@ -392,7 +392,8 @@ acldefault(char *relname, AclId ownerid)
/*
- * Add or replace an item in an ACL array.
+ * Add or replace an item in an ACL array. The result is a modified copy;
+ * the input object is not changed.
*
* NB: caller is responsible for having detoasted the input ACL, if needed.
*/
@@ -402,8 +403,7 @@ aclinsert3(Acl *old_acl, AclItem *mod_aip, unsigned modechg)
Acl *new_acl;
AclItem *old_aip,
*new_aip;
- int src,
- dst,
+ int dst,
num;
/* These checks for null input are probably dead code, but... */
@@ -431,14 +431,14 @@ aclinsert3(Acl *old_acl, AclItem *mod_aip, unsigned modechg)
if (dst < num && aclitemeq(mod_aip, old_aip + dst))
{
- /* modify in-place */
+ /* found a match, so modify existing item */
new_acl = makeacl(num);
new_aip = ACL_DAT(new_acl);
memcpy((char *) new_acl, (char *) old_acl, ACL_SIZE(old_acl));
- src = dst;
}
else
{
+ /* need to insert a new item */
new_acl = makeacl(num + 1);
new_aip = ACL_DAT(new_acl);
if (dst == 0)
@@ -460,20 +460,21 @@ aclinsert3(Acl *old_acl, AclItem *mod_aip, unsigned modechg)
(char *) (old_aip + dst),
(num - dst) * sizeof(AclItem));
}
+ /* initialize the new entry with no permissions */
new_aip[dst].ai_id = mod_aip->ai_id;
new_aip[dst].ai_idtype = mod_aip->ai_idtype;
+ new_aip[dst].ai_mode = 0;
num++; /* set num to the size of new_acl */
- src = 0; /* if add or del, start from world entry */
}
/* apply the permissions mod */
switch (modechg)
{
case ACL_MODECHG_ADD:
- new_aip[dst].ai_mode = old_aip[src].ai_mode | mod_aip->ai_mode;
+ new_aip[dst].ai_mode |= mod_aip->ai_mode;
break;
case ACL_MODECHG_DEL:
- new_aip[dst].ai_mode = old_aip[src].ai_mode & ~mod_aip->ai_mode;
+ new_aip[dst].ai_mode &= ~mod_aip->ai_mode;
break;
case ACL_MODECHG_EQL:
new_aip[dst].ai_mode = mod_aip->ai_mode;
@@ -487,16 +488,10 @@ aclinsert3(Acl *old_acl, AclItem *mod_aip, unsigned modechg)
*/
if (new_aip[dst].ai_mode == 0 && dst > 0)
{
- int i;
-
- for (i = dst + 1; i < num; i++)
- {
- new_aip[i - 1].ai_id = new_aip[i].ai_id;
- new_aip[i - 1].ai_idtype = new_aip[i].ai_idtype;
- new_aip[i - 1].ai_mode = new_aip[i].ai_mode;
- }
+ memmove((char *) (new_aip + dst),
+ (char *) (new_aip + dst + 1),
+ (num - dst - 1) * sizeof(AclItem));
ARR_DIMS(new_acl)[0] = num - 1;
- /* Adjust also the array size because it is used for memcpy */
ARR_SIZE(new_acl) -= sizeof(AclItem);
}
diff --git a/src/include/utils/acl.h b/src/include/utils/acl.h
index 02e6094c51e..2ea98d5cb6e 100644
--- a/src/include/utils/acl.h
+++ b/src/include/utils/acl.h
@@ -7,7 +7,7 @@
* Portions Copyright (c) 1996-2001, PostgreSQL Global Development Group
* Portions Copyright (c) 1994, Regents of the University of California
*
- * $Id: acl.h,v 1.32 2001/05/27 09:59:30 petere Exp $
+ * $Id: acl.h,v 1.33 2001/06/05 19:34:56 tgl Exp $
*
* NOTES
* For backward-compatibility purposes we have to allow there
@@ -164,7 +164,7 @@ typedef ArrayType IdList;
#define ACLCHECK_NO_CLASS 2
#define ACLCHECK_NOT_OWNER 3
-/* warning messages. set these in aclchk.c. */
+/* error messages (index by ACL_CHECK_* result code). set in aclchk.c. */
extern char *aclcheck_error_strings[];
/*
@@ -201,10 +201,12 @@ extern AclId get_grosysid(char *groname);
extern char *get_groname(AclId grosysid);
extern int32 pg_aclcheck(char *relname, Oid userid, AclMode mode);
-extern int32 pg_ownercheck(Oid userid, const char *value, int cacheid);
-extern int32 pg_func_ownercheck(Oid userid, char *funcname,
- int nargs, Oid *arglist);
-extern int32 pg_aggr_ownercheck(Oid userid, char *aggname,
- Oid basetypeID);
+
+extern bool pg_ownercheck(Oid userid, const char *name, int cacheid);
+extern bool pg_oper_ownercheck(Oid userid, Oid oprid);
+extern bool pg_func_ownercheck(Oid userid, char *funcname,
+ int nargs, Oid *arglist);
+extern bool pg_aggr_ownercheck(Oid userid, char *aggname,
+ Oid basetypeID);
#endif /* ACL_H */
--
GitLab