Re: Add more sanity checks around callers of changeDependencyFor()

On Thu, Jul 06, 2023 at 10:09:20AM -0700, Andres Freund wrote:
> Well, it adds an exploitation opportunity. If other functions in the extension
> reference the original location (explicitly or via search_path), somebody else
> can create a function there, which might be called from a more privileged
> context. Obviously permissions limit the likelihood of this being a real
> issue.

Yeah..

> I also don't think pg_dump will dump the changed schema, which means a
> dump/restore leads to a different schema - IMO something to avoid.

Yes, you're right here. The function dumped is restored in the same
schema as the extension. For instance:
psql postgres << EOF
CREATE SCHEMA test_func_dep1;
CREATE SCHEMA test_func_dep2;
CREATE EXTENSION test_ext_req_schema1 SCHEMA test_func_dep1;
ALTER FUNCTION test_func_dep1.dep_req1() SET SCHEMA test_func_dep2;
EOF
pg_dump -f dump.sql postgres
createdb popo
psql -f dump.sql popo
psql -c '\dx+ test_ext_req_schema1' popo

Objects in extension "test_ext_req_schema1"
Object description
------------------------------------
function test_func_dep1.dep_req1()
(1 row)

I am honestly not sure how much restrictions we should have here, as
this could hurt anybody relying on the existing behavior, as well, if
there are any. (It seems that that schema modification restrictions
for extension objects would need to go through
ExecAlterObjectSchemaStmt().)

Anyway, I think that I'll just go ahead and fix the SET SCHEMA bug, as
that's wrong as it stands. Regarding these restrictions, perhaps
something could be done on HEAD, though it impacts usability IMO.
--
Michael