| From: | Matheus Alcantara <matheusssilv97(at)gmail(dot)com> |
|---|---|
| To: | Amit Langote <amitlangote09(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Cc: | rmt(at)lists(dot)postgresql(dot)org |
| Subject: | Re: RI fastpath misses checking EXECUTE on functions |
| Date: | 2026-09-24 14:45:54 |
| Message-ID: | 21aff2c6-33d2-458a-b7c6-c84b21e568da@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On 24/09/26 00:42, Amit Langote wrote:
> Hi,
>
> It was pointed out to me offlist by Nikolay Samokhvalov that the RI
> fast path (ri_FastPathCheck) doesn't check EXECUTE privilege on the
> functions it invokes on the FK values. There are two such functions:
> the equality operator's function, which goes into the ScanKey the
> index AM evaluates, and the implicit cast function, which is used when
> the FK value must be converted to the PK column's type. In the SPI
> path's case, because the operator appears in the generated query's
> WHERE clause and the cast as $1::pktype, ExecutorStart checked EXECUTE
> on both as part of initializing the query, as the referenced table's
> owner. So while I added ri_CheckPermissions() to parallel the
> executor's ExecCheckPermissions(), I missed the non-relation
> permission checks that occur as part of expression initialization.
>
> The attached patch adds ri_CheckFunctionPermissions(), which does
> object_aclcheck(ProcedureRelationId, ..., ACL_EXECUTE) plus
> InvokeFunctionExecuteHook() for each of the two functions, and calls
> it once the fast-path metadata is populated. That puts the checks at
> the same point as the existing relation permission check.
>
> Regression tests are included for both functions. They reach the fast
> path through per-row validation of a new constraint, and repeat each
> case against a partitioned referenced table, which takes the SPI path,
> to establish that the two behave identically.
>
> I'll add an open item. I would like to get this committed by EOW if
> there are no comments.
>
I take a look on the patch, it looks good to me. Just a minor
comment:
ri_CheckFunctionPermissions(riinfo, fpmeta) passes both when fpmeta
== riinfo->fpmeta. I'm wondering if we could just pass riinfo?
IIUC this patch only fix the case for FastPath without batching right?
Since batching is still on master, I'm wondering if we could also fix
it. See attached patch (v2-0001 is your v1-0001).
--
Matheus Alcantara
EDB: https://www.enterprisedb.com
| Attachment | Content-Type | Size |
|---|---|---|
| v2-0001-Check-EXECUTE-privilege-on-functions-invoked-by-t.patch | text/plain | 13.3 KB |
| v2-0002-Check-EXECUTE-on-functions-invoked-by-the-batched.patch | text/plain | 5.6 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Trakshan Mishra | 2026-09-24 14:46:16 | Re: Re: [PATCH] Fix segmentation fault caused by reentrancy in RI_Fkey_cascade_del (ri_triggers.c) |
| Previous Message | Sami Imseih | 2026-09-24 14:43:14 | Re: REPACK (CONCURRENTLY) loses missing values of columns added without a rewrite |