MIME-Version: 1.0
References: <19510-953c48eaf669350b@postgresql.org>
 <CADHzGZQ9qM-JrTN+mBRHapDYVKymPV=E39nV5aB_N+sSTR=35A@mail.gmail.com>
 <3539886.1780617976@sss.pgh.pa.us>
In-Reply-To: <3539886.1780617976@sss.pgh.pa.us>
From: Amjad Shahzad <amjadshahzad2000@gmail.com>
Date: Fri, 5 Jun 2026 05:29:16 +0500
Message-ID: 
 <CADHzGZTsdeo9pBVbeSTn2p3rgRdWN3ijiDU5QkyfLK6+MxihwQ@mail.gmail.com>
Subject: Re: BUG #19510: refint.c: SQL injection via unquoted identifier
 arguments in check_primary_key and check_foreign_key
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-bugs@lists.postgresql.org
Content-Type: multipart/alternative; boundary="00000000000034c777065376c126"
Archived-At: 
 <https://www.postgresql.org/message-id/CADHzGZTsdeo9pBVbeSTn2p3rgRdWN3ijiDU5QkyfLK6%2BMxihwQ%40mail.gmail.com>
Precedence: bulk

--00000000000034c777065376c126
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi Tom,

Thank you for the detailed review and for explaining the security team's
earlier discussion.

You're absolutely right, I missed the schema-qualified name case.
quote_identifier('myschema.mytable') would produce "myschema.mytable"
treating the dot as part of the identifier, which would silently break
existing valid triggers. That's a real regression, and I withdrew the patch=
.

Understood that the security team's position is documentation rather than a
code fix given these constraints. Thanks again for taking the
time to explain the reasoning.

Regards,
Amjad

On Fri, Jun 5, 2026 at 5:06=E2=80=AFAM Tom Lane <tgl@sss.pgh.pa.us> wrote:

> Amjad Shahzad <amjadshahzad2000@gmail.com> writes:
> > Patch attached for the issue reported above.
>
> I don't think we can just blindly "quote_identifier" all these
> strings.  As an example, suppose somebody has set the relname
> argument of a trigger to 'myschema.mytable'.  Their code works
> fine today, and is perfectly secure, and your patch would break it.
> Mixed-case identifiers are another trouble spot where quoting
> could change the meaning of valid code.
>
> The pgsql-security team already discussed these issues while preparing
> the recent CVEs in this area, and concluded that the only workable
> path forward is to add documentation explaining that these arguments
> are handled as fragments of SQL query text.  So any required quoting
> is up to the calling application.  Fortunately, trigger arguments are
> not the sort of thing that's likely to be taken blindly from untrusted
> input.
>
>                         regards, tom lane
>

--00000000000034c777065376c126
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr">Hi Tom,<br><br>Thank you for the detailed review and for e=
xplaining the security team&#39;s earlier discussion.<br><br>You&#39;re abs=
olutely right, I missed the schema-qualified name case.<br>quote_identifier=
(&#39;myschema.mytable&#39;) would produce &quot;myschema.mytable&quot; tre=
ating the dot as part of the identifier, which would silently break<br>exis=
ting valid triggers. That&#39;s a real regression, and I withdrew the patch=
.<br><br>Understood that the security team&#39;s position is documentation =
rather than a code fix given these constraints. Thanks again for taking the=
<br>time to explain the reasoning.<br><br>Regards,<br>Amjad</div><br><div c=
lass=3D"gmail_quote gmail_quote_container"><div dir=3D"ltr" class=3D"gmail_=
attr">On Fri, Jun 5, 2026 at 5:06=E2=80=AFAM Tom Lane &lt;<a href=3D"mailto=
:tgl@sss.pgh.pa.us">tgl@sss.pgh.pa.us</a>&gt; wrote:<br></div><blockquote c=
lass=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px soli=
d rgb(204,204,204);padding-left:1ex">Amjad Shahzad &lt;<a href=3D"mailto:am=
jadshahzad2000@gmail.com" target=3D"_blank">amjadshahzad2000@gmail.com</a>&=
gt; writes:<br>
&gt; Patch attached for the issue reported above.<br>
<br>
I don&#39;t think we can just blindly &quot;quote_identifier&quot; all thes=
e<br>
strings.=C2=A0 As an example, suppose somebody has set the relname<br>
argument of a trigger to &#39;myschema.mytable&#39;.=C2=A0 Their code works=
<br>
fine today, and is perfectly secure, and your patch would break it.<br>
Mixed-case identifiers are another trouble spot where quoting<br>
could change the meaning of valid code.<br>
<br>
The pgsql-security team already discussed these issues while preparing<br>
the recent CVEs in this area, and concluded that the only workable<br>
path forward is to add documentation explaining that these arguments<br>
are handled as fragments of SQL query text.=C2=A0 So any required quoting<b=
r>
is up to the calling application.=C2=A0 Fortunately, trigger arguments are<=
br>
not the sort of thing that&#39;s likely to be taken blindly from untrusted<=
br>
input.<br>
<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 regards, tom lane<br>
</blockquote></div>

--00000000000034c777065376c126--