Обсуждение: Leakproofness of texteq()/textne()

Currently, texteq() and textne() are marked leakproof, while
sibling operations such as textlt() are not.  The argument
for that was that those two functions depend only on memcmp()
so they can be seen to be safe, whereas it's a whole lot less
clear that text_cmp() should be considered leakproof.

Of course, the addition of nondeterministic collations has
utterly obliterated that argument: it's now possible for
texteq() to call text_cmp(), so if the latter is leaky then
the former certainly must be considered so as well.

Seems like we have two choices:

1. Remove the leakproof marking on texteq()/textne().  This
would, unfortunately, be catastrophic for performance in
a lot of scenarios where leakproofness matters.

2. Fix text_cmp to actually be leakproof, whereupon we might
as well mark all the remaining btree comparison operators
leakproof.

I think #2 is pretty obviously the superior answer, if we
can do it.

ISTM we can't ship v12 without dealing with this one way
or the other, so I'll go add an open item.

Comments?

            regards, tom lane

I wrote:
> Seems like we have two choices:
> 1. Remove the leakproof marking on texteq()/textne().  This
> would, unfortunately, be catastrophic for performance in
> a lot of scenarios where leakproofness matters.
> 2. Fix text_cmp to actually be leakproof, whereupon we might
> as well mark all the remaining btree comparison operators
> leakproof.
> I think #2 is pretty obviously the superior answer, if we
> can do it.

After burrowing down further, it's visibly the case that
text_cmp and varstr_cmp don't leak in the sense of actually
reporting any part of their input strings.  What they do do,
in some code paths, is things like

        ereport(ERROR,
                (errmsg("could not convert string to UTF-16: error code %lu",
                        GetLastError())));

So this seems to mostly be a question of interpretation:
does an error like this represent enough of an information
leak to violate the promises of leakproofness?  I'm not sure
that this error is really capable of disclosing any information
about the input strings.  (Note that this error occurs only if
we failed to convert UTF8 to UTF16, which probably could only
happen if the input isn't valid UTF8, which would mean a failure
of encoding checking somewhere up the line.)

There are also various pallocs and such that could fail, which
perhaps could be argued to disclose the lengths of the input
strings.  But that hazard exists already inside PG_GETARG_TEXT_PP;
if you want to complain about that, then functions like byteaeq()
aren't legitimately leakproof either.

On the whole I'm prepared to say that these aren't leakproofness
violations, but it would depend a lot on how strict you want to be.

            regards, tom lane

On Thu, Sep 12, 2019 at 12:19 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> After burrowing down further, it's visibly the case that
> text_cmp and varstr_cmp don't leak in the sense of actually
> reporting any part of their input strings.  What they do do,
> in some code paths, is things like
>
>         ereport(ERROR,
>                 (errmsg("could not convert string to UTF-16: error code %lu",
>                         GetLastError())));

Is this possible? I mean, I'm sure it could happen if the data's
corrupted, but we ought to have validated it on the way into the
database. But maybe this code path also gets used for non-Unicode
encodings?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Robert Haas <robertmhaas@gmail.com> writes:
> On Thu, Sep 12, 2019 at 12:19 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> After burrowing down further, it's visibly the case that
>> text_cmp and varstr_cmp don't leak in the sense of actually
>> reporting any part of their input strings.  What they do do,
>> in some code paths, is things like
>>         ereport(ERROR,
>>                 (errmsg("could not convert string to UTF-16: error code %lu",
>>                         GetLastError())));

> Is this possible? I mean, I'm sure it could happen if the data's
> corrupted, but we ought to have validated it on the way into the
> database. But maybe this code path also gets used for non-Unicode
> encodings?

Nope, the above is inside

#ifdef WIN32
        /* Win32 does not have UTF-8, so we need to map to UTF-16 */
        if (GetDatabaseEncoding() == PG_UTF8
            && (!mylocale || mylocale->provider == COLLPROVIDER_LIBC))

I agree with your point that this is a shouldn't-happen corner case.
The question boils down to, if it *does* happen, does that constitute
a meaningful information leak?  Up to now we've taken quite a hard
line about what leakproofness means, so deciding that varstr_cmp
is leakproof would constitute moving the goalposts a bit.  They'd
still be in the same stadium, though, IMO.

Another approach would be to try to remove these failure cases,
but I don't really see how we'd do that.

            regards, tom lane

I wrote:
> I agree with your point that this is a shouldn't-happen corner case.
> The question boils down to, if it *does* happen, does that constitute
> a meaningful information leak?  Up to now we've taken quite a hard
> line about what leakproofness means, so deciding that varstr_cmp
> is leakproof would constitute moving the goalposts a bit.  They'd
> still be in the same stadium, though, IMO.

For most of us it might be more meaningful to look at the non-Windows
code paths, for which the question reduces to what we think of this:

                    UErrorCode    status;

                    status = U_ZERO_ERROR;
                    result = ucol_strcollUTF8(mylocale->info.icu.ucol,
                                              arg1, len1,
                                              arg2, len2,
                                              &status);
                    if (U_FAILURE(status))
                        ereport(ERROR,
                                (errmsg("collation failed: %s", u_errorName(status))));

which, as it happens, is also a UTF8-encoding-only code path.
Can this throw an error in practice, and if so does that
constitute a meaningful information leak?  (For bonus points:
is this error report up to project standards?)

Thumbing through the list of UErrorCode values, it seems like the only
ones that are applicable here and aren't internal-error cases are
U_INVALID_CHAR_FOUND and the like, so that this boils down to "one of
the strings contains a character that ICU can't cope with".  Maybe that's
impossible except with a pre-existing encoding violation, or maybe not.

In any case, from a purely theoretical viewpoint, such an error message
*does* constitute a leak of information about the input strings.  Whether
it's a usable leak is very debatable, but that's basically what we've
got to decide.

            regards, tom lane

On Thu, Sep 12, 2019 at 1:38 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> In any case, from a purely theoretical viewpoint, such an error message
> *does* constitute a leak of information about the input strings.  Whether
> it's a usable leak is very debatable, but that's basically what we've
> got to decide.

I'm pretty content to ignore information leaks that can only happen if
the database is corrupt anyway.  If that's moving the goalposts at
all, it's about a quarter-inch.  I mean, a slightly differently
corrupted varlena would could crash the database entirely.

I wouldn't feel comfortable with ignoring information leaks that can
happen with some valid strings but not others. That sounds like
exactly the sort of information leak that we must prevent. The user
can write arbitrary stuff in their query, potentially transforming
strings so that the result hits the ERROR iff the original string had
some arbitrary property P for which they wish to test. Allowing that
sounds no different than deciding that int4div is leakproof, which it
sure isn't.

However, I wonder if there's any realistic case outside of an encoding
conversion where such failures can occur. I would expect, perhaps
naively, that the set of characters that can be represented by UTF-16
is the same set as can be represented by UTF-8.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Robert Haas <robertmhaas@gmail.com> writes:
> I wouldn't feel comfortable with ignoring information leaks that can
> happen with some valid strings but not others. That sounds like
> exactly the sort of information leak that we must prevent. The user
> can write arbitrary stuff in their query, potentially transforming
> strings so that the result hits the ERROR iff the original string had
> some arbitrary property P for which they wish to test.

Agreed.

> However, I wonder if there's any realistic case outside of an encoding
> conversion where such failures can occur. I would expect, perhaps
> naively, that the set of characters that can be represented by UTF-16
> is the same set as can be represented by UTF-8.

Unless Microsoft did something weird, that doesn't seem very likely.
I'm more worried about the possibility that ICU contains weird exception
cases that will make it fail to compare specific strings.  Noting
that ucnv_toUChars has an error indicator but ucol_strcoll does not,
it seems like again any such cases are going to boil down to
encoding conversion problems.

However, if there is some character C that makes ICU misbehave like
that, we are going to have problems with indexing strings containing C,
whether we think varstr_cmp is leaky or not.  So I'm not sure that
focusing our attention on leakiness is a helpful way to proceed.

            regards, tom lane

On Thu, Sep 12, 2019 at 5:19 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> However, if there is some character C that makes ICU misbehave like
> that, we are going to have problems with indexing strings containing C,
> whether we think varstr_cmp is leaky or not.  So I'm not sure that
> focusing our attention on leakiness is a helpful way to proceed.

That seems like a compelling argument to me.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Greetings,

* Robert Haas (robertmhaas@gmail.com) wrote:
> On Thu, Sep 12, 2019 at 5:19 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > However, if there is some character C that makes ICU misbehave like
> > that, we are going to have problems with indexing strings containing C,
> > whether we think varstr_cmp is leaky or not.  So I'm not sure that
> > focusing our attention on leakiness is a helpful way to proceed.
>
> That seems like a compelling argument to me.

Agreed.

Thanks,

Stephen

Stephen Frost <sfrost@snowman.net> writes:
> * Robert Haas (robertmhaas@gmail.com) wrote:
>> On Thu, Sep 12, 2019 at 5:19 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> However, if there is some character C that makes ICU misbehave like
>>> that, we are going to have problems with indexing strings containing C,
>>> whether we think varstr_cmp is leaky or not.  So I'm not sure that
>>> focusing our attention on leakiness is a helpful way to proceed.

>> That seems like a compelling argument to me.

> Agreed.

So it seems that the consensus is that it's okay to mark these
functions leakproof, because if any of the errors they throw
are truly reachable for other than data-corruption reasons,
we would wish to try to prevent such errors.  (Maybe through
upstream validity checks?  Hard to say how we'd do it exactly,
when we don't have an idea what the problem is.)

My inclination is to do the proleakproof changes in HEAD, but
not touch v12.  The inconsistency in leakproof markings in v12
is annoying but it's not a regression or security hazard, so
I'm thinking it's not worth a late catversion bump to fix.

Thoughts?

            regards, tom lane

On 2019-09-16 06:24, Tom Lane wrote:
> So it seems that the consensus is that it's okay to mark these
> functions leakproof, because if any of the errors they throw
> are truly reachable for other than data-corruption reasons,
> we would wish to try to prevent such errors.  (Maybe through
> upstream validity checks?  Hard to say how we'd do it exactly,
> when we don't have an idea what the problem is.)

Yeah, it seems like as we expand our Unicode capabilities, we will see
more cases like "it could fail here in theory, but it shouldn't happen
for normal data", and the answer can't be to call all that untrusted or
leaky.  It's the job of the database software to sort that out.
Obviously, it will require careful evaluation in each case.

> My inclination is to do the proleakproof changes in HEAD, but
> not touch v12.  The inconsistency in leakproof markings in v12
> is annoying but it's not a regression or security hazard, so
> I'm thinking it's not worth a late catversion bump to fix.

Sounds good, unless we do another catversion bump.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
> On 2019-09-16 06:24, Tom Lane wrote:
>> So it seems that the consensus is that it's okay to mark these
>> functions leakproof, because if any of the errors they throw
>> are truly reachable for other than data-corruption reasons,
>> we would wish to try to prevent such errors.  (Maybe through
>> upstream validity checks?  Hard to say how we'd do it exactly,
>> when we don't have an idea what the problem is.)

> Yeah, it seems like as we expand our Unicode capabilities, we will see
> more cases like "it could fail here in theory, but it shouldn't happen
> for normal data", and the answer can't be to call all that untrusted or
> leaky.  It's the job of the database software to sort that out.
> Obviously, it will require careful evaluation in each case.

Here's a proposed patch to mark functions that depend on varstr_cmp
as leakproof.  I think we can apply this to HEAD and then close the
open item as "won't fix for v12".

            regards, tom lane

diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c
index d36156f..8c18294 100644
--- a/src/backend/utils/adt/varlena.c
+++ b/src/backend/utils/adt/varlena.c
@@ -1463,6 +1463,12 @@ check_collation_set(Oid collid)
  *    to allow null-termination for inputs to strcoll().
  * Returns an integer less than, equal to, or greater than zero, indicating
  * whether arg1 is less than, equal to, or greater than arg2.
+ *
+ * Note: many functions that depend on this are marked leakproof; therefore,
+ * avoid reporting the actual contents of the input when throwing errors.
+ * All errors herein should be things that can't happen except on corrupt
+ * data, anyway; otherwise we will have trouble with indexing strings that
+ * would cause them.
  */
 int
 varstr_cmp(const char *arg1, int len1, const char *arg2, int len2, Oid collid)
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index e6645f1..5e69dee 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -681,44 +681,44 @@
   proname => 'nameeqtext', proleakproof => 't', prorettype => 'bool',
   proargtypes => 'name text', prosrc => 'nameeqtext' },
 { oid => '241',
-  proname => 'namelttext', prorettype => 'bool', proargtypes => 'name text',
-  prosrc => 'namelttext' },
+  proname => 'namelttext', proleakproof => 't', prorettype => 'bool',
+  proargtypes => 'name text', prosrc => 'namelttext' },
 { oid => '242',
-  proname => 'nameletext', prorettype => 'bool', proargtypes => 'name text',
-  prosrc => 'nameletext' },
+  proname => 'nameletext', proleakproof => 't', prorettype => 'bool',
+  proargtypes => 'name text', prosrc => 'nameletext' },
 { oid => '243',
-  proname => 'namegetext', prorettype => 'bool', proargtypes => 'name text',
-  prosrc => 'namegetext' },
+  proname => 'namegetext', proleakproof => 't', prorettype => 'bool',
+  proargtypes => 'name text', prosrc => 'namegetext' },
 { oid => '244',
-  proname => 'namegttext', prorettype => 'bool', proargtypes => 'name text',
-  prosrc => 'namegttext' },
+  proname => 'namegttext', proleakproof => 't', prorettype => 'bool',
+  proargtypes => 'name text', prosrc => 'namegttext' },
 { oid => '245',
   proname => 'namenetext', proleakproof => 't', prorettype => 'bool',
   proargtypes => 'name text', prosrc => 'namenetext' },
 { oid => '246', descr => 'less-equal-greater',
-  proname => 'btnametextcmp', prorettype => 'int4', proargtypes => 'name text',
-  prosrc => 'btnametextcmp' },
+  proname => 'btnametextcmp', proleakproof => 't', prorettype => 'int4',
+  proargtypes => 'name text', prosrc => 'btnametextcmp' },
 { oid => '247',
   proname => 'texteqname', proleakproof => 't', prorettype => 'bool',
   proargtypes => 'text name', prosrc => 'texteqname' },
 { oid => '248',
-  proname => 'textltname', prorettype => 'bool', proargtypes => 'text name',
-  prosrc => 'textltname' },
+  proname => 'textltname', proleakproof => 't', prorettype => 'bool',
+  proargtypes => 'text name', prosrc => 'textltname' },
 { oid => '249',
-  proname => 'textlename', prorettype => 'bool', proargtypes => 'text name',
-  prosrc => 'textlename' },
+  proname => 'textlename', proleakproof => 't', prorettype => 'bool',
+  proargtypes => 'text name', prosrc => 'textlename' },
 { oid => '250',
-  proname => 'textgename', prorettype => 'bool', proargtypes => 'text name',
-  prosrc => 'textgename' },
+  proname => 'textgename', proleakproof => 't', prorettype => 'bool',
+  proargtypes => 'text name', prosrc => 'textgename' },
 { oid => '251',
-  proname => 'textgtname', prorettype => 'bool', proargtypes => 'text name',
-  prosrc => 'textgtname' },
+  proname => 'textgtname', proleakproof => 't', prorettype => 'bool',
+  proargtypes => 'text name', prosrc => 'textgtname' },
 { oid => '252',
   proname => 'textnename', proleakproof => 't', prorettype => 'bool',
   proargtypes => 'text name', prosrc => 'textnename' },
 { oid => '253', descr => 'less-equal-greater',
-  proname => 'bttextnamecmp', prorettype => 'int4', proargtypes => 'text name',
-  prosrc => 'bttextnamecmp' },
+  proname => 'bttextnamecmp', proleakproof => 't', prorettype => 'int4',
+  proargtypes => 'text name', prosrc => 'bttextnamecmp' },

 { oid => '266', descr => 'concatenate name and oid',
   proname => 'nameconcatoid', prorettype => 'name', proargtypes => 'name oid',
@@ -1002,14 +1002,14 @@
   proname => 'btcharcmp', proleakproof => 't', prorettype => 'int4',
   proargtypes => 'char char', prosrc => 'btcharcmp' },
 { oid => '359', descr => 'less-equal-greater',
-  proname => 'btnamecmp', prorettype => 'int4', proargtypes => 'name name',
-  prosrc => 'btnamecmp' },
+  proname => 'btnamecmp', proleakproof => 't', prorettype => 'int4',
+  proargtypes => 'name name', prosrc => 'btnamecmp' },
 { oid => '3135', descr => 'sort support',
   proname => 'btnamesortsupport', prorettype => 'void',
   proargtypes => 'internal', prosrc => 'btnamesortsupport' },
 { oid => '360', descr => 'less-equal-greater',
-  proname => 'bttextcmp', prorettype => 'int4', proargtypes => 'text text',
-  prosrc => 'bttextcmp' },
+  proname => 'bttextcmp', proleakproof => 't', prorettype => 'int4',
+  proargtypes => 'text text', prosrc => 'bttextcmp' },
 { oid => '3255', descr => 'sort support',
   proname => 'bttextsortsupport', prorettype => 'void',
   proargtypes => 'internal', prosrc => 'bttextsortsupport' },
@@ -1230,11 +1230,11 @@
   proargmodes => '{v}', prosrc => 'pg_num_nonnulls' },

 { oid => '458', descr => 'larger of two',
-  proname => 'text_larger', prorettype => 'text', proargtypes => 'text text',
-  prosrc => 'text_larger' },
+  proname => 'text_larger', proleakproof => 't', prorettype => 'text',
+  proargtypes => 'text text', prosrc => 'text_larger' },
 { oid => '459', descr => 'smaller of two',
-  proname => 'text_smaller', prorettype => 'text', proargtypes => 'text text',
-  prosrc => 'text_smaller' },
+  proname => 'text_smaller', proleakproof => 't', prorettype => 'text',
+  proargtypes => 'text text', prosrc => 'text_smaller' },

 { oid => '460', descr => 'I/O',
   proname => 'int8in', prorettype => 'int8', proargtypes => 'cstring',
@@ -1334,17 +1334,17 @@
   prosrc => 'int28' },

 { oid => '655',
-  proname => 'namelt', prorettype => 'bool', proargtypes => 'name name',
-  prosrc => 'namelt' },
+  proname => 'namelt', proleakproof => 't', prorettype => 'bool',
+  proargtypes => 'name name', prosrc => 'namelt' },
 { oid => '656',
-  proname => 'namele', prorettype => 'bool', proargtypes => 'name name',
-  prosrc => 'namele' },
+  proname => 'namele', proleakproof => 't', prorettype => 'bool',
+  proargtypes => 'name name', prosrc => 'namele' },
 { oid => '657',
-  proname => 'namegt', prorettype => 'bool', proargtypes => 'name name',
-  prosrc => 'namegt' },
+  proname => 'namegt', proleakproof => 't', prorettype => 'bool',
+  proargtypes => 'name name', prosrc => 'namegt' },
 { oid => '658',
-  proname => 'namege', prorettype => 'bool', proargtypes => 'name name',
-  prosrc => 'namege' },
+  proname => 'namege', proleakproof => 't', prorettype => 'bool',
+  proargtypes => 'name name', prosrc => 'namege' },
 { oid => '659',
   proname => 'namene', proleakproof => 't', prorettype => 'bool',
   proargtypes => 'name name', prosrc => 'namene' },
@@ -1451,17 +1451,17 @@
   proargtypes => 'circle point', prosrc => 'dist_cpoint' },

 { oid => '740',
-  proname => 'text_lt', prorettype => 'bool', proargtypes => 'text text',
-  prosrc => 'text_lt' },
+  proname => 'text_lt', proleakproof => 't', prorettype => 'bool',
+  proargtypes => 'text text', prosrc => 'text_lt' },
 { oid => '741',
-  proname => 'text_le', prorettype => 'bool', proargtypes => 'text text',
-  prosrc => 'text_le' },
+  proname => 'text_le', proleakproof => 't', prorettype => 'bool',
+  proargtypes => 'text text', prosrc => 'text_le' },
 { oid => '742',
-  proname => 'text_gt', prorettype => 'bool', proargtypes => 'text text',
-  prosrc => 'text_gt' },
+  proname => 'text_gt', proleakproof => 't', prorettype => 'bool',
+  proargtypes => 'text text', prosrc => 'text_gt' },
 { oid => '743',
-  proname => 'text_ge', prorettype => 'bool', proargtypes => 'text text',
-  prosrc => 'text_ge' },
+  proname => 'text_ge', proleakproof => 't', prorettype => 'bool',
+  proargtypes => 'text text', prosrc => 'text_ge' },

 { oid => '745', descr => 'current user name',
   proname => 'current_user', provolatile => 's', prorettype => 'name',
@@ -2051,29 +2051,29 @@
   proname => 'bpchareq', proleakproof => 't', prorettype => 'bool',
   proargtypes => 'bpchar bpchar', prosrc => 'bpchareq' },
 { oid => '1049',
-  proname => 'bpcharlt', prorettype => 'bool', proargtypes => 'bpchar bpchar',
-  prosrc => 'bpcharlt' },
+  proname => 'bpcharlt', proleakproof => 't', prorettype => 'bool',
+  proargtypes => 'bpchar bpchar', prosrc => 'bpcharlt' },
 { oid => '1050',
-  proname => 'bpcharle', prorettype => 'bool', proargtypes => 'bpchar bpchar',
-  prosrc => 'bpcharle' },
+  proname => 'bpcharle', proleakproof => 't', prorettype => 'bool',
+  proargtypes => 'bpchar bpchar', prosrc => 'bpcharle' },
 { oid => '1051',
-  proname => 'bpchargt', prorettype => 'bool', proargtypes => 'bpchar bpchar',
-  prosrc => 'bpchargt' },
+  proname => 'bpchargt', proleakproof => 't', prorettype => 'bool',
+  proargtypes => 'bpchar bpchar', prosrc => 'bpchargt' },
 { oid => '1052',
-  proname => 'bpcharge', prorettype => 'bool', proargtypes => 'bpchar bpchar',
-  prosrc => 'bpcharge' },
+  proname => 'bpcharge', proleakproof => 't', prorettype => 'bool',
+  proargtypes => 'bpchar bpchar', prosrc => 'bpcharge' },
 { oid => '1053',
   proname => 'bpcharne', proleakproof => 't', prorettype => 'bool',
   proargtypes => 'bpchar bpchar', prosrc => 'bpcharne' },
 { oid => '1063', descr => 'larger of two',
-  proname => 'bpchar_larger', prorettype => 'bpchar',
+  proname => 'bpchar_larger', proleakproof => 't', prorettype => 'bpchar',
   proargtypes => 'bpchar bpchar', prosrc => 'bpchar_larger' },
 { oid => '1064', descr => 'smaller of two',
-  proname => 'bpchar_smaller', prorettype => 'bpchar',
+  proname => 'bpchar_smaller', proleakproof => 't', prorettype => 'bpchar',
   proargtypes => 'bpchar bpchar', prosrc => 'bpchar_smaller' },
 { oid => '1078', descr => 'less-equal-greater',
-  proname => 'bpcharcmp', prorettype => 'int4', proargtypes => 'bpchar bpchar',
-  prosrc => 'bpcharcmp' },
+  proname => 'bpcharcmp', proleakproof => 't', prorettype => 'int4',
+  proargtypes => 'bpchar bpchar', prosrc => 'bpcharcmp' },
 { oid => '3328', descr => 'sort support',
   proname => 'bpchar_sortsupport', prorettype => 'void',
   proargtypes => 'internal', prosrc => 'bpchar_sortsupport' },
@@ -6541,38 +6541,38 @@
   proargtypes => 'float8 float8', prosrc => 'aggregate_dummy' },

 { oid => '2160',
-  proname => 'text_pattern_lt', prorettype => 'bool',
+  proname => 'text_pattern_lt', proleakproof => 't', prorettype => 'bool',
   proargtypes => 'text text', prosrc => 'text_pattern_lt' },
 { oid => '2161',
-  proname => 'text_pattern_le', prorettype => 'bool',
+  proname => 'text_pattern_le', proleakproof => 't', prorettype => 'bool',
   proargtypes => 'text text', prosrc => 'text_pattern_le' },
 { oid => '2163',
-  proname => 'text_pattern_ge', prorettype => 'bool',
+  proname => 'text_pattern_ge', proleakproof => 't', prorettype => 'bool',
   proargtypes => 'text text', prosrc => 'text_pattern_ge' },
 { oid => '2164',
-  proname => 'text_pattern_gt', prorettype => 'bool',
+  proname => 'text_pattern_gt', proleakproof => 't', prorettype => 'bool',
   proargtypes => 'text text', prosrc => 'text_pattern_gt' },
 { oid => '2166', descr => 'less-equal-greater',
-  proname => 'bttext_pattern_cmp', prorettype => 'int4',
+  proname => 'bttext_pattern_cmp', proleakproof => 't', prorettype => 'int4',
   proargtypes => 'text text', prosrc => 'bttext_pattern_cmp' },
 { oid => '3332', descr => 'sort support',
   proname => 'bttext_pattern_sortsupport', prorettype => 'void',
   proargtypes => 'internal', prosrc => 'bttext_pattern_sortsupport' },

 { oid => '2174',
-  proname => 'bpchar_pattern_lt', prorettype => 'bool',
+  proname => 'bpchar_pattern_lt', proleakproof => 't', prorettype => 'bool',
   proargtypes => 'bpchar bpchar', prosrc => 'bpchar_pattern_lt' },
 { oid => '2175',
-  proname => 'bpchar_pattern_le', prorettype => 'bool',
+  proname => 'bpchar_pattern_le', proleakproof => 't', prorettype => 'bool',
   proargtypes => 'bpchar bpchar', prosrc => 'bpchar_pattern_le' },
 { oid => '2177',
-  proname => 'bpchar_pattern_ge', prorettype => 'bool',
+  proname => 'bpchar_pattern_ge', proleakproof => 't', prorettype => 'bool',
   proargtypes => 'bpchar bpchar', prosrc => 'bpchar_pattern_ge' },
 { oid => '2178',
-  proname => 'bpchar_pattern_gt', prorettype => 'bool',
+  proname => 'bpchar_pattern_gt', proleakproof => 't', prorettype => 'bool',
   proargtypes => 'bpchar bpchar', prosrc => 'bpchar_pattern_gt' },
 { oid => '2180', descr => 'less-equal-greater',
-  proname => 'btbpchar_pattern_cmp', prorettype => 'int4',
+  proname => 'btbpchar_pattern_cmp', proleakproof => 't', prorettype => 'int4',
   proargtypes => 'bpchar bpchar', prosrc => 'btbpchar_pattern_cmp' },
 { oid => '3333', descr => 'sort support',
   proname => 'btbpchar_pattern_sortsupport', prorettype => 'void',
diff --git a/src/test/regress/expected/opr_sanity.out b/src/test/regress/expected/opr_sanity.out
index 33c058f..c19740e 100644
--- a/src/test/regress/expected/opr_sanity.out
+++ b/src/test/regress/expected/opr_sanity.out
@@ -526,9 +526,19 @@ int42ge(integer,smallint)
 oideq(oid,oid)
 oidne(oid,oid)
 nameeqtext(name,text)
+namelttext(name,text)
+nameletext(name,text)
+namegetext(name,text)
+namegttext(name,text)
 namenetext(name,text)
+btnametextcmp(name,text)
 texteqname(text,name)
+textltname(text,name)
+textlename(text,name)
+textgename(text,name)
+textgtname(text,name)
 textnename(text,name)
+bttextnamecmp(text,name)
 float4eq(real,real)
 float4ne(real,real)
 float4lt(real,real)
@@ -559,8 +569,12 @@ btfloat4cmp(real,real)
 btfloat8cmp(double precision,double precision)
 btoidcmp(oid,oid)
 btcharcmp("char","char")
+btnamecmp(name,name)
+bttextcmp(text,text)
 cash_cmp(money,money)
 btoidvectorcmp(oidvector,oidvector)
+text_larger(text,text)
+text_smaller(text,text)
 int8eq(bigint,bigint)
 int8ne(bigint,bigint)
 int8lt(bigint,bigint)
@@ -574,6 +588,10 @@ int84gt(bigint,integer)
 int84le(bigint,integer)
 int84ge(bigint,integer)
 oidvectorne(oidvector,oidvector)
+namelt(name,name)
+namele(name,name)
+namegt(name,name)
+namege(name,name)
 namene(name,name)
 oidvectorlt(oidvector,oidvector)
 oidvectorle(oidvector,oidvector)
@@ -582,6 +600,10 @@ oidvectorge(oidvector,oidvector)
 oidvectorgt(oidvector,oidvector)
 oidlt(oid,oid)
 oidle(oid,oid)
+text_lt(text,text)
+text_le(text,text)
+text_gt(text,text)
+text_ge(text,text)
 macaddr_eq(macaddr,macaddr)
 macaddr_lt(macaddr,macaddr)
 macaddr_le(macaddr,macaddr)
@@ -611,7 +633,14 @@ network_ne(inet,inet)
 network_cmp(inet,inet)
 lseg_eq(lseg,lseg)
 bpchareq(character,character)
+bpcharlt(character,character)
+bpcharle(character,character)
+bpchargt(character,character)
+bpcharge(character,character)
 bpcharne(character,character)
+bpchar_larger(character,character)
+bpchar_smaller(character,character)
+bpcharcmp(character,character)
 date_eq(date,date)
 date_lt(date,date)
 date_le(date,date)
@@ -707,6 +736,16 @@ timestamp_lt(timestamp without time zone,timestamp without time zone)
 timestamp_le(timestamp without time zone,timestamp without time zone)
 timestamp_ge(timestamp without time zone,timestamp without time zone)
 timestamp_gt(timestamp without time zone,timestamp without time zone)
+text_pattern_lt(text,text)
+text_pattern_le(text,text)
+text_pattern_ge(text,text)
+text_pattern_gt(text,text)
+bttext_pattern_cmp(text,text)
+bpchar_pattern_lt(character,character)
+bpchar_pattern_le(character,character)
+bpchar_pattern_ge(character,character)
+bpchar_pattern_gt(character,character)
+btbpchar_pattern_cmp(character,character)
 btint48cmp(integer,bigint)
 btint84cmp(bigint,integer)
 btint24cmp(smallint,integer)
@@ -1322,8 +1361,6 @@ WHERE o1.oprnegate = o2.oid AND p1.oid = o1.oprcode AND p2.oid = o2.oprcode AND

 -- Btree comparison operators' functions should have the same volatility
 -- and leakproofness markings as the associated comparison support function.
--- As of Postgres 12, the only exceptions are that textual equality functions
--- are marked leakproof but textual comparison/inequality functions are not.
 SELECT pp.oid::regprocedure as proc, pp.provolatile as vp, pp.proleakproof as lp,
        po.oid::regprocedure as opr, po.provolatile as vo, po.proleakproof as lo
 FROM pg_proc pp, pg_proc po, pg_operator o, pg_amproc ap, pg_amop ao
@@ -1336,16 +1373,9 @@ WHERE pp.oid = ap.amproc AND po.oid = o.oprcode AND o.oid = ao.amopopr AND
     (pp.provolatile != po.provolatile OR
      pp.proleakproof != po.proleakproof)
 ORDER BY 1;
-                   proc                    | vp | lp |              opr              | vo | lo
--------------------------------------------+----+----+-------------------------------+----+----
- btnametextcmp(name,text)                  | i  | f  | nameeqtext(name,text)         | i  | t
- bttextnamecmp(text,name)                  | i  | f  | texteqname(text,name)         | i  | t
- btnamecmp(name,name)                      | i  | f  | nameeq(name,name)             | i  | t
- bttextcmp(text,text)                      | i  | f  | texteq(text,text)             | i  | t
- bpcharcmp(character,character)            | i  | f  | bpchareq(character,character) | i  | t
- bttext_pattern_cmp(text,text)             | i  | f  | texteq(text,text)             | i  | t
- btbpchar_pattern_cmp(character,character) | i  | f  | bpchareq(character,character) | i  | t
-(7 rows)
+ proc | vp | lp | opr | vo | lo
+------+----+----+-----+----+----
+(0 rows)

 -- **************** pg_aggregate ****************
 -- Look for illegal values in pg_aggregate fields.
diff --git a/src/test/regress/sql/opr_sanity.sql b/src/test/regress/sql/opr_sanity.sql
index 1227ef7..624bea4 100644
--- a/src/test/regress/sql/opr_sanity.sql
+++ b/src/test/regress/sql/opr_sanity.sql
@@ -806,8 +806,6 @@ WHERE o1.oprnegate = o2.oid AND p1.oid = o1.oprcode AND p2.oid = o2.oprcode AND

 -- Btree comparison operators' functions should have the same volatility
 -- and leakproofness markings as the associated comparison support function.
--- As of Postgres 12, the only exceptions are that textual equality functions
--- are marked leakproof but textual comparison/inequality functions are not.
 SELECT pp.oid::regprocedure as proc, pp.provolatile as vp, pp.proleakproof as lp,
        po.oid::regprocedure as opr, po.provolatile as vo, po.proleakproof as lo
 FROM pg_proc pp, pg_proc po, pg_operator o, pg_amproc ap, pg_amop ao