| From: | Ewan Young <kdbase(dot)hack(at)gmail(dot)com> |
|---|---|
| To: | Pg Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Cc: | Ayush Tiwari <ayushtiwari(dot)slg01(at)gmail(dot)com>, Richard Guo <guofenglinux(at)gmail(dot)com> |
| Subject: | Re: plpython: NULL pointer dereference on broken sequence objects |
| Date: | 2026-07-07 10:08:42 |
| Message-ID: | CAON2xHMRVCmjHP-FdpBN-SOWkBg3jmXouu0HWV+R+EQmNA-GBg@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi,
While looking at the transforms hardened by commit 8612f0b7ce0
("plpython: Fix NULL pointer dereferences for broken sequence and
mapping objects"), I noticed one spot that seems to have been left out.
That commit added a "pcount < 0" check after PyMapping_Size() in
PLyMapping_ToJsonbValue(), but the sibling PLySequence_ToJsonbValue()
still uses the result of PySequence_Size() without checking it:
pcount = PySequence_Size(obj);
pushJsonbValue(jsonb_state, WJB_BEGIN_ARRAY, NULL);
PG_TRY();
{
for (i = 0; i < pcount; i++)
...
PySequence_Size() returns -1, with a Python exception set, for an
object that passes PySequence_Check() but whose length cannot be
determined -- e.g. one whose len() raises, or a legacy sequence
that implements getitem() but no len() at all. Since the
negative count is used directly as the loop bound, the loop is simply
skipped and the transform silently produces an empty jsonb array
instead of reporting the error.
This isn't limited to deliberately broken objects. A plain old-style
sequence (only getitem) that Python happily iterates is silently
turned into []:
CREATE FUNCTION f() RETURNS jsonb
TRANSFORM FOR TYPE jsonb LANGUAGE plpython3u AS $$
class Seq:
def __getitem__(self, i):
if i < 3:
return i * 10
raise IndexError
return Seq()
$$;
-- list(Seq()) in Python is [0, 10, 20], but:
SELECT f();
f
----
[]
Worse, PySequence_Size() leaves the Python error indicator set, so a
subsequent unrelated PL/Python call in the same session can fail with
a confusing "returned a result with an error set" SystemError that has
nothing to do with the actual culprit.
The attached patch fixes it the same way the mapping side was fixed,
by checking for a negative result and surfacing the underlying Python
exception via PLy_elog(). After the patch the examples above raise a
proper error:
ERROR: could not get size of Python sequence
DETAIL: TypeError: object of type 'Seq' has no len()
and the leftover exception no longer poisons later calls. It also adds
regression tests alongside the ones from 8612f0b, and passes
"make installcheck". Like that commit, I think this should be
back-patched through 14.
On Mon, Jun 29, 2026 at 11:01 AM Richard Guo <guofenglinux(at)gmail(dot)com> wrote:
>
> On Fri, Jun 26, 2026 at 4:22 PM Ayush Tiwari
> <ayushtiwari(dot)slg01(at)gmail(dot)com> wrote:
> > On Fri, 26 Jun 2026 at 11:35, Richard Guo <guofenglinux(at)gmail(dot)com> wrote:
> >> Right. Those functions have the same issue as PySequence_GetItem().
> >> The attached v2 patch fixes them all.
>
> > Thanks for the update! v2 LGTM.
>
> Thanks for the review. Pushed and back-patched down to v14.
>
> - Richard
>
>
--
Regards,
Ewan Young
| Attachment | Content-Type | Size |
|---|---|---|
| v1-0001-jsonb_plpython-Check-for-negative-PySequence_Size-result.patch | application/octet-stream | 4.7 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Kouber Saparev | 2026-07-07 10:13:59 | Re: BF mamba failure |
| Previous Message | Hans Buschmann | 2026-07-07 09:59:35 | Increased SECURITY RISCS from omitting some compikler options when building PG with meson; e.g. -fcf-protection=full |