Remove PushOverrideSearchPath() and PopOverrideSearchPath().
authorNoah Misch <noah@leadboat.com>
Tue, 1 Aug 2023 00:04:47 +0000 (17:04 -0700)
committerNoah Misch <noah@leadboat.com>
Tue, 1 Aug 2023 00:04:47 +0000 (17:04 -0700)
Since commit 681d9e4621aac0a9c71364b6f54f00f6d8c4337f, they have no in-tree
calls.  Any new calls would introduce security vulnerabilities like the one
fixed in that commit.

Alexander Lakhin, reviewed by Aleksander Alekseev.

Discussion: https://postgr.es/m/8ffb4650-52c4-6a81-38fc-8f99be981130@gmail.com

src/backend/catalog/namespace.c
src/backend/commands/extension.c
src/include/catalog/namespace.h
src/tools/pgindent/typedefs.list

index 69ab1b8e4b898e3cf8e912826e256eed1d96965e..f51a13aa56f09eb60eb9e7f72050b524be59751f 100644 (file)
@@ -67,9 +67,7 @@
  * may be included:
  *
  * 1. If a TEMP table namespace has been initialized in this session, it
- * is implicitly searched first.  (The only time this doesn't happen is
- * when we are obeying an override search path spec that says not to use the
- * temp namespace, or the temp namespace is included in the explicit list.)
+ * is implicitly searched first.
  *
  * 2. The system catalog namespace is always searched.  If the system
  * namespace is present in the explicit path then it will be searched in
  * namespace (if it exists), preceded by the user's personal namespace
  * (if one exists).
  *
- * We support a stack of "override" search path settings for use within
- * specific sections of backend code.  namespace_search_path is ignored
- * whenever the override stack is nonempty.  activeSearchPath is always
- * the actually active path; it points either to the search list of the
- * topmost stack entry, or to baseSearchPath which is the list derived
- * from namespace_search_path.
+ * activeSearchPath is always the actually active path; it points to
+ * to baseSearchPath which is the list derived from namespace_search_path.
  *
  * If baseSearchPathValid is false, then baseSearchPath (and other
  * derived variables) need to be recomputed from namespace_search_path.
  * We mark it invalid upon an assignment to namespace_search_path or receipt
  * of a syscache invalidation event for pg_namespace.  The recomputation
- * is done during the next non-overridden lookup attempt.  Note that an
- * override spec is never subject to recomputation.
+ * is done during the next lookup attempt.
  *
  * Any namespaces mentioned in namespace_search_path that are not readable
  * by the current user ID are simply left out of baseSearchPath; so
@@ -161,17 +154,6 @@ static Oid namespaceUser = InvalidOid;
 /* The above four values are valid only if baseSearchPathValid */
 static bool baseSearchPathValid = true;
 
-/* Override requests are remembered in a stack of OverrideStackEntry structs */
-
-typedef struct
-{
-   List       *searchPath;     /* the desired search path */
-   Oid         creationNamespace;  /* the desired creation namespace */
-   int         nestLevel;      /* subtransaction nesting level */
-} OverrideStackEntry;
-
-static List *overrideStack = NIL;
-
 /*
  * myTempNamespace is InvalidOid until and unless a TEMP namespace is set up
  * in a particular backend session (this happens when a CREATE TEMP TABLE
@@ -3392,8 +3374,7 @@ SetTempNamespaceState(Oid tempNamespaceId, Oid tempToastNamespaceId)
 
 
 /*
- * GetOverrideSearchPath - fetch current search path definition in form
- * used by PushOverrideSearchPath.
+ * GetOverrideSearchPath - fetch current search path definition.
  *
  * The result structure is allocated in the specified memory context
  * (which might or might not be equal to CurrentMemoryContext); but any
@@ -3512,132 +3493,6 @@ OverrideSearchPathMatchesCurrent(OverrideSearchPath *path)
    return true;
 }
 
-/*
- * PushOverrideSearchPath - temporarily override the search path
- *
- * Do not use this function; almost any usage introduces a security
- * vulnerability.  It exists for the benefit of legacy code running in
- * non-security-sensitive environments.
- *
- * We allow nested overrides, hence the push/pop terminology.  The GUC
- * search_path variable is ignored while an override is active.
- *
- * It's possible that newpath->useTemp is set but there is no longer any
- * active temp namespace, if the path was saved during a transaction that
- * created a temp namespace and was later rolled back.  In that case we just
- * ignore useTemp.  A plausible alternative would be to create a new temp
- * namespace, but for existing callers that's not necessary because an empty
- * temp namespace wouldn't affect their results anyway.
- *
- * It's also worth noting that other schemas listed in newpath might not
- * exist anymore either.  We don't worry about this because OIDs that match
- * no existing namespace will simply not produce any hits during searches.
- */
-void
-PushOverrideSearchPath(OverrideSearchPath *newpath)
-{
-   OverrideStackEntry *entry;
-   List       *oidlist;
-   Oid         firstNS;
-   MemoryContext oldcxt;
-
-   /*
-    * Copy the list for safekeeping, and insert implicitly-searched
-    * namespaces as needed.  This code should track recomputeNamespacePath.
-    */
-   oldcxt = MemoryContextSwitchTo(TopMemoryContext);
-
-   oidlist = list_copy(newpath->schemas);
-
-   /*
-    * Remember the first member of the explicit list.
-    */
-   if (oidlist == NIL)
-       firstNS = InvalidOid;
-   else
-       firstNS = linitial_oid(oidlist);
-
-   /*
-    * Add any implicitly-searched namespaces to the list.  Note these go on
-    * the front, not the back; also notice that we do not check USAGE
-    * permissions for these.
-    */
-   if (newpath->addCatalog)
-       oidlist = lcons_oid(PG_CATALOG_NAMESPACE, oidlist);
-
-   if (newpath->addTemp && OidIsValid(myTempNamespace))
-       oidlist = lcons_oid(myTempNamespace, oidlist);
-
-   /*
-    * Build the new stack entry, then insert it at the head of the list.
-    */
-   entry = (OverrideStackEntry *) palloc(sizeof(OverrideStackEntry));
-   entry->searchPath = oidlist;
-   entry->creationNamespace = firstNS;
-   entry->nestLevel = GetCurrentTransactionNestLevel();
-
-   overrideStack = lcons(entry, overrideStack);
-
-   /* And make it active. */
-   activeSearchPath = entry->searchPath;
-   activeCreationNamespace = entry->creationNamespace;
-   activeTempCreationPending = false;  /* XXX is this OK? */
-
-   /*
-    * We always increment activePathGeneration when pushing/popping an
-    * override path.  In current usage, these actions always change the
-    * effective path state, so there's no value in checking to see if it
-    * didn't change.
-    */
-   activePathGeneration++;
-
-   MemoryContextSwitchTo(oldcxt);
-}
-
-/*
- * PopOverrideSearchPath - undo a previous PushOverrideSearchPath
- *
- * Any push during a (sub)transaction will be popped automatically at abort.
- * But it's caller error if a push isn't popped in normal control flow.
- */
-void
-PopOverrideSearchPath(void)
-{
-   OverrideStackEntry *entry;
-
-   /* Sanity checks. */
-   if (overrideStack == NIL)
-       elog(ERROR, "bogus PopOverrideSearchPath call");
-   entry = (OverrideStackEntry *) linitial(overrideStack);
-   if (entry->nestLevel != GetCurrentTransactionNestLevel())
-       elog(ERROR, "bogus PopOverrideSearchPath call");
-
-   /* Pop the stack and free storage. */
-   overrideStack = list_delete_first(overrideStack);
-   list_free(entry->searchPath);
-   pfree(entry);
-
-   /* Activate the next level down. */
-   if (overrideStack)
-   {
-       entry = (OverrideStackEntry *) linitial(overrideStack);
-       activeSearchPath = entry->searchPath;
-       activeCreationNamespace = entry->creationNamespace;
-       activeTempCreationPending = false;  /* XXX is this OK? */
-   }
-   else
-   {
-       /* If not baseSearchPathValid, this is useless but harmless */
-       activeSearchPath = baseSearchPath;
-       activeCreationNamespace = baseCreationNamespace;
-       activeTempCreationPending = baseTempCreationPending;
-   }
-
-   /* As above, the generation always increments. */
-   activePathGeneration++;
-}
-
-
 /*
  * get_collation_oid - find a collation by possibly qualified name
  *
@@ -3794,10 +3649,6 @@ recomputeNamespacePath(void)
    bool        pathChanged;
    MemoryContext oldcxt;
 
-   /* Do nothing if an override search spec is active. */
-   if (overrideStack)
-       return;
-
    /* Do nothing if path is already valid. */
    if (baseSearchPathValid && namespaceUser == roleid)
        return;
@@ -3936,10 +3787,7 @@ recomputeNamespacePath(void)
 
    /*
     * Bump the generation only if something actually changed.  (Notice that
-    * what we compared to was the old state of the base path variables; so
-    * this does not deal with the situation where we have just popped an
-    * override path and restored the prior state of the base path.  Instead
-    * we rely on the override-popping logic to have bumped the generation.)
+    * what we compared to was the old state of the base path variables.)
     */
    if (pathChanged)
        activePathGeneration++;
@@ -4142,29 +3990,6 @@ AtEOXact_Namespace(bool isCommit, bool parallel)
        myTempNamespaceSubID = InvalidSubTransactionId;
    }
 
-   /*
-    * Clean up if someone failed to do PopOverrideSearchPath
-    */
-   if (overrideStack)
-   {
-       if (isCommit)
-           elog(WARNING, "leaked override search path");
-       while (overrideStack)
-       {
-           OverrideStackEntry *entry;
-
-           entry = (OverrideStackEntry *) linitial(overrideStack);
-           overrideStack = list_delete_first(overrideStack);
-           list_free(entry->searchPath);
-           pfree(entry);
-       }
-       /* If not baseSearchPathValid, this is useless but harmless */
-       activeSearchPath = baseSearchPath;
-       activeCreationNamespace = baseCreationNamespace;
-       activeTempCreationPending = baseTempCreationPending;
-       /* Always bump generation --- see note in recomputeNamespacePath */
-       activePathGeneration++;
-   }
 }
 
 /*
@@ -4179,7 +4004,6 @@ void
 AtEOSubXact_Namespace(bool isCommit, SubTransactionId mySubid,
                      SubTransactionId parentSubid)
 {
-   OverrideStackEntry *entry;
 
    if (myTempNamespaceSubID == mySubid)
    {
@@ -4205,51 +4029,6 @@ AtEOSubXact_Namespace(bool isCommit, SubTransactionId mySubid,
            MyProc->tempNamespaceId = InvalidOid;
        }
    }
-
-   /*
-    * Clean up if someone failed to do PopOverrideSearchPath
-    */
-   while (overrideStack)
-   {
-       entry = (OverrideStackEntry *) linitial(overrideStack);
-       if (entry->nestLevel < GetCurrentTransactionNestLevel())
-           break;
-       if (isCommit)
-           elog(WARNING, "leaked override search path");
-       overrideStack = list_delete_first(overrideStack);
-       list_free(entry->searchPath);
-       pfree(entry);
-       /* Always bump generation --- see note in recomputeNamespacePath */
-       activePathGeneration++;
-   }
-
-   /* Activate the next level down. */
-   if (overrideStack)
-   {
-       entry = (OverrideStackEntry *) linitial(overrideStack);
-       activeSearchPath = entry->searchPath;
-       activeCreationNamespace = entry->creationNamespace;
-       activeTempCreationPending = false;  /* XXX is this OK? */
-
-       /*
-        * It's probably unnecessary to bump generation here, but this should
-        * not be a performance-critical case, so better to be over-cautious.
-        */
-       activePathGeneration++;
-   }
-   else
-   {
-       /* If not baseSearchPathValid, this is useless but harmless */
-       activeSearchPath = baseSearchPath;
-       activeCreationNamespace = baseCreationNamespace;
-       activeTempCreationPending = baseTempCreationPending;
-
-       /*
-        * If we popped an override stack entry, then we already bumped the
-        * generation above.  If we did not, then the above assignments did
-        * nothing and we need not bump the generation.
-        */
-   }
 }
 
 /*
index 9a2ee1c600825f2f187f80a95a3c226364c124fc..4cc994ca31e0b77970fe784d0c6afc523e8750db 100644 (file)
@@ -967,11 +967,6 @@ execute_extension_script(Oid extensionOid, ExtensionControlFile *control,
     * searched anyway.  (Listing pg_catalog explicitly in a non-first
     * position would be bad for security.)  Finally add pg_temp to ensure
     * that temp objects can't take precedence over others.
-    *
-    * Note: it might look tempting to use PushOverrideSearchPath for this,
-    * but we cannot do that.  We have to actually set the search_path GUC in
-    * case the extension script examines or changes it.  In any case, the
-    * GUC_ACTION_SAVE method is just as convenient.
     */
    initStringInfo(&pathbuf);
    appendStringInfoString(&pathbuf, quote_identifier(schemaName));
index f64a0ec26b97be02902008e13765e0a20fee6df3..93e0c1234539b2f5537a4fc78e7d70451f9628c9 100644 (file)
@@ -167,8 +167,6 @@ extern void ResetTempTableNamespace(void);
 extern OverrideSearchPath *GetOverrideSearchPath(MemoryContext context);
 extern OverrideSearchPath *CopyOverrideSearchPath(OverrideSearchPath *path);
 extern bool OverrideSearchPathMatchesCurrent(OverrideSearchPath *path);
-extern void PushOverrideSearchPath(OverrideSearchPath *newpath);
-extern void PopOverrideSearchPath(void);
 
 extern Oid get_collation_oid(List *collname, bool missing_ok);
 extern Oid get_conversion_oid(List *conname, bool missing_ok);
index ab97f1794a88baa8a15f589d3e2c12386d4cb04f..dc296afb63d2506a3f4ef41f1670cbf14fca3662 100644 (file)
@@ -1687,7 +1687,6 @@ OutputPluginCallbacks
 OutputPluginOptions
 OutputPluginOutputType
 OverrideSearchPath
-OverrideStackEntry
 OverridingKind
 PACE_HEADER
 PACL