Making the ENUM operators LEAKPROOF

Started by Laurenz Albe4 months ago5 messageshackers
Beta feature

Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.

appliessuccessCI history

You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:

docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t139513
psql -h localhost -U postgres

Built from patchset v2 (message #2), August 23, 2026 at 12:43 AM.

Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:

git clone --branch t139513_2 https://github.com/hackorum-dev/postgres.git

In a checkout you already have, add the fork once:

git remote add hackorum https://github.com/hackorum-dev/postgres.git

then, for this patchset and every later one:

git fetch hackorum t139513_2 && git checkout t139513_2

Patchset v2 (message #2) is on t139513_2

Jump to latest
#1Laurenz Albe
laurenz.albe@cybertec.at

This is an attempt to get the operators from the "enum_ops" operator
class LEAKPROOF. There have been previous discussion about that, see
[1]: /messages/by-id/flat/31042.1546194242@sss.pgh.pa.us
(see [2]/messages/by-id/flat/2811772.0XtDgEdalL@peanuts2), so I won't discuss it here. The hard part is to prove that
the operators are actually LEAKPROOF.

I think that we should discuss the goal posts first. What is required
to prove LEAKPROOF?

I hope that we can all agree that the only information side channels
in the enum_* functions are error messages. If yes, it will suffice
to discuss the potential error messages and what they can reveal about
the compared values.

I think that any discussion about the requirements of LEAKPROOF needs
to consider the other side of the coin: the guarantees given by
row-level security and security barrier views. It makes no sense to
hold LEAKPROOF to higher standards than what we are ready to guarantee
for RLS and security barrier views.

Now [4]/messages/by-id/flat/3a60be45e7a89f50d166dba49553950d6b8a97f5.camel@cybertec.at and the discussion on the security list left me with the
impression that row-level security is not intended to be safe against
attacks by a user who can run arbitrary SQL statements. For example,
it is easy to work around RLS using EXPLAIN (ANALYZE)). For security
barrier views, that limitation is explicitly documented in the final
paragraph of
https://www.postgresql.org/docs/current/rules-privileges.html

Consequently, I propose that for proving a function to be LEAKPROOF,
we only consider information leaks that can be triggered by a user
supplying parameters to an SQL statement run by an application.
For example, small differences in function execution time cannot be
exploited that way; they will be dominated by variations in the
application execution time and network latency.
Also, any attacks that require catalog manipulations or other
superuser-only modifications should be disregarded. Superusers can
always subvert security rules.

One particular case that I want to discuss is out-of-memory conditions.
Some of the OOM error messages show the allocation size that failed
(dshash.c, mbutils.c, dsa.c and mcxt.c), which could leak some
information about the data involved. My feeling is that that is
something you cannot exploit just by using an application; you'd have
to cause just the right memory pressure to make small allocations fail.
(If we decide to err on the side of caution here, I'd actually prefer
to have the error messages changed to not reveal the allocation size.)

------

Now, to a discussion of the ENUM comparison functions.

enum_eq() is clearly leakproof, and we have to concentrate on
enum_cmp(), and there on enum_cmp_internal(), which has all the tricky
parts.

There is a call to SearchSysCache1() and --- in load_enum_cache_data(),
which is called from compare_values_of_enum() --- an index scan on
pg_enum. Both of these allocate memory, and both can fail in the
face of data corruption. I hope we can agree that data corruption is
not a scenario we have to consider. The memory allocated is to cache
the ENUM values, and their size and definition is already public
information in pg_enum, so I don't consider that a leak.
SearchCatCacheInternal() has some DEBUG2 messages, but they don't
output any catalog values.

Apart from that, I find the following error messages in the code:

- "invalid internal value for enum: %u" in enum_cmp_internal():
That can only happen if somebody manages to call an ENUM
comparison function with something that isn't an ENUM.
But you cannot use SQL to call the ENUM comparison functions with
something that isn't an ENUM.

- "%s is not an enum" in load_enum_cache_data(): again that can only
be reached if somebody calls the function with something that is
not an ENUM, which I argue you cannot do using the comparison
functions.

- "enum value %u not found in cache for enum %s" in
compare_values_of_enum(): you can only trigger an error message
if one of the values you compare is a value that doesn't exist
for the enum. I don't think you can trigger that error via SQL.
The only way this error could leak the (numeric!) value of an
ENUM value in the database is if that value is illegal, that is,
if there is data corruption.

My conclusion from all that is that it should be safe to set the
ENUM comparison functions LEAKPROOF.

To recapitulate, I would like to hear your opinion about these
potentially contentions questions:

- Do you agree that it is sufficient that an argument for
LEAKPROOFness need only consider attacks through an application
(since row-level security and security barries views are not
designed to withstand attacks by arbitrary SQL statements)?

- Do you think that we can disregard out-of-memory errors when
proving LEAKPROOFness?

- Do you agree with my conclusion that the ENUM comparison
operators should be LEAKPROOF? Did I miss anything?

Yours,
Laurenz Albe

[1]: /messages/by-id/flat/31042.1546194242@sss.pgh.pa.us
[2]: /messages/by-id/flat/2811772.0XtDgEdalL@peanuts2
[3]: /messages/by-id/flat/CAMxA3rtdJ2OdsPm8pqUKFXh=EyB-7Ypyjny=ZPXoGZEK2nHTKQ@mail.gmail.com
[4]: /messages/by-id/flat/3a60be45e7a89f50d166dba49553950d6b8a97f5.camel@cybertec.at

#2Laurenz Albe
laurenz.albe@cybertec.at
In reply to: Laurenz Albe (#1)
Re: Making the ENUM operators LEAKPROOF

On Wed, 2026-04-29 at 17:32 +0200, Laurenz Albe wrote:

This is an attempt to get the operators from the "enum_ops" operator
class LEAKPROOF.

[...]

One particular case that I want to discuss is out-of-memory conditions.
Some of the OOM error messages show the allocation size that failed
(dshash.c, mbutils.c, dsa.c and mcxt.c), which could leak some
information about the data involved. My feeling is that that is
something you cannot exploit just by using an application; you'd have
to cause just the right memory pressure to make small allocations fail.
(If we decide to err on the side of caution here, I'd actually prefer
to have the error messages changed to not reveal the allocation size.)

I have become even less worried about that case after seeing that the
"text" comparison functions are leakproof. With "text", one could argue
that you could trigger an OOM error by comparing with a value you don't
know, if that value has to be detoasted. That way, you could know the
length of the unknown string. If "text" is considered safe enough, I
don't think we need to worry about enum_cmp(), which doesn't even have
that problem.

I prepared a patch so that I can add the thread to the next commitfest.

Yours,
Laurenz Albe

Attachments:

t139513_2
v1-0001-Mark-the-enum-comparison-functions-as-leakproof.patchtext/x-patch; charset=UTF-8; name=v1-0001-Mark-the-enum-comparison-functions-as-leakproof.patchDownload+14-8
#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Laurenz Albe (#1)
Re: Making the ENUM operators LEAKPROOF

Laurenz Albe <laurenz.albe@cybertec.at> writes:

This is an attempt to get the operators from the "enum_ops" operator
class LEAKPROOF.

I think we should reject this idea. Our standard for marking
functions leakproof has always included a requirement that the amount
of code involved be small enough that one can immediately confirm that
there is no information leak. I don't think that enum_cmp() can meet
that standard; the length of your argument already proves the point.
The issue here is not really "can I convince myself that it's safe
today?". It's more about "am I willing to bet that no future code
change is going to unintentionally break the property?". The more
code involved, the riskier that bet.

I know that you are going to say "but text_cmp covers a lot of
ground, including code (in libc or ICU) that isn't even ours".
To my mind, we made an exception for text_cmp because the performance
consequences of not making it leakproof were unacceptable.
I do not think enum_cmp() can clear that bar either.

FWIW, I do agree that marking enum_eq leakproof is safe. How
many of the use-cases you are worried about would be satisfied
with that?

regards, tom lane

#4Laurenz Albe
laurenz.albe@cybertec.at
In reply to: Tom Lane (#3)
Re: Making the ENUM operators LEAKPROOF

On Wed, 2026-07-01 at 18:33 -0400, Tom Lane wrote:

Laurenz Albe <laurenz.albe@cybertec.at> writes:

This is an attempt to get the operators from the "enum_ops" operator
class LEAKPROOF.

I think we should reject this idea. Our standard for marking
functions leakproof has always included a requirement that the amount
of code involved be small enough that one can immediately confirm that
there is no information leak. I don't think that enum_cmp() can meet
that standard; the length of your argument already proves the point.
The issue here is not really "can I convince myself that it's safe
today?". It's more about "am I willing to bet that no future code
change is going to unintentionally break the property?". The more
code involved, the riskier that bet.

Yes, future code changes could be a risk.
However, I don't think it particularly likely that any error
message added will output one of the arguments of the comparison
function: all the tricky parts in that function deal with catalog
lookups or memory allocation for a cache.

What bothers me in general is that we hold leakproofness to
a standard that borders on the paranoid (which is not necessarily
wrong with security features), but the promises we make for
row-level security are rather low (EXPLAIN ANALYZE subverts it)
and we have rejected improvements in that area.
I am worried that that makes row-level security much less useful
in practice than it could and should be.

I know that you are going to say "but text_cmp covers a lot of
ground, including code (in libc or ICU) that isn't even ours".
To my mind, we made an exception for text_cmp because the performance
consequences of not making it leakproof were unacceptable.
I do not think enum_cmp() can clear that bar either.

I won't dispute that "text" is more important than "enum".

But enums are important enough for one of our customers that
they had me look into getting enum comparisons leakproof.

Andreas, do you want to comment?

FWIW, I do agree that marking enum_eq leakproof is safe. How
many of the use-cases you are worried about would be satisfied
with that?

Only equality comparisons are important, but in [1]/messages/by-id/641899.1750095380@sss.pgh.pa.us you made
the argument that something could substitute enum_cmp() for
enum_eq(), so I thought it would be necessary to get both
leakproof.

Yours,
Laurenz Albe

[1]: /messages/by-id/641899.1750095380@sss.pgh.pa.us

#5Andreas Lind
andreaslindpetersen@gmail.com
In reply to: Laurenz Albe (#4)
Re: Making the ENUM operators LEAKPROOF

On Mon, Jul 20, 2026 at 2:21 PM Laurenz Albe <laurenz.albe@cybertec.at>
wrote:

But enums are important enough for one of our customers that
they had me look into getting enum comparisons leakproof.

Andreas, do you want to comment?

Hi! Yeah, I help maintain an application that makes extensive use of
ENUM types, including in indexes and composite primary keys. Adopting
row-level security was somewhat of a struggle, as chronicled in [1]/messages/by-id/CAMxA3rtdJ2OdsPm8pqUKFXh=EyB-7Ypyjny=ZPXoGZEK2nHTKQ@mail.gmail.com.

These are the functions whose lack of LEAKPROOFness prevented
predicates from being used as index conditions while row-level security
was active:

* enum_eq(anyenum, anyenum) from enum_ops
* arraycontained(anyarray, anyarray) from gin_array_ops
* arraycontains(anyarray, anyarray) from gin_array_ops
* arrayoverlap(anyarray, anyarray) from gin_array_ops

We eventually found workarounds for all of these issues. The main
reason for posting here, and for involving Laurenz, was to document
our experience and hopefully help others avoid the same pitfalls.

It seemed like getting those functions reviewed and LEAKPROOFed would
be the most straightforward course of action. Zooming out, though,
I hope we can agree that it's worthy goal for these core Postgres
features (RLS, ENUM types, B-tree indexes, and GIN indexes) to compose
well "out of the box" without subtle and potentially severe performance
regressions.

I suggested several other approaches in [1]/messages/by-id/CAMxA3rtdJ2OdsPm8pqUKFXh=EyB-7Ypyjny=ZPXoGZEK2nHTKQ@mail.gmail.com, and I would be willing to
work further on them if there is interest. At the same time, I do not
know enough about the relevant PostgreSQL internals to judge which
direction would make the best long-term design.

My earlier proposal was partly an attempt to nerd-snipe people with
more planner and security-barrier experience into reconsidering the
broader problem. :-)

Does any of you have a good solution in mind?

Best regards,
Andreas Lind

[1]: /messages/by-id/CAMxA3rtdJ2OdsPm8pqUKFXh=EyB-7Ypyjny=ZPXoGZEK2nHTKQ@mail.gmail.com
/messages/by-id/CAMxA3rtdJ2OdsPm8pqUKFXh=EyB-7Ypyjny=ZPXoGZEK2nHTKQ@mail.gmail.com