Thread

Commits

  1. plpython: Fix NULL pointer dereferences for broken sequence and mapping objects

  1. plpython: NULL pointer dereference on broken sequence objects

    Richard Guo <guofenglinux@gmail.com> — 2026-06-25T08:49:31Z

    While looking into the recent plperl NULL pointer dereference issue,
    which ended up as 4015abe14, I found a similar issue in plpython, with
    the help of an LLM tool (Claude 4.8).
    
    There are 6 callers of PySequence_GetItem() in plpython, and none of
    them checks the returned result before using it.  PySequence_GetItem()
    can return NULL whenever an element cannot be fetched, so an object
    that claims a length it cannot actually deliver is enough to crash the
    backend.
    
    For example:
    
      CREATE FUNCTION test() RETURNS int[] AS $$
      class C:
          def __len__(self):
              return 2
          def __getitem__(self, i):
              raise ValueError('boom')
      return C()
      $$ LANGUAGE plpython3u;
    
      SELECT test();   -- crashes
    
    
    The attached patch checks the result of PySequence_GetItem() in each
    place and errors out if it is NULL.
    
    - Richard
    
  2. Re: plpython: NULL pointer dereference on broken sequence objects

    Ayush Tiwari <ayushtiwari.slg01@gmail.com> — 2026-06-25T13:05:01Z

    Hi,
    
    On Thu, 25 Jun 2026 at 14:19, Richard Guo <guofenglinux@gmail.com> wrote:
    
    > While looking into the recent plperl NULL pointer dereference issue,
    > which ended up as 4015abe14, I found a similar issue in plpython, with
    > the help of an LLM tool (Claude 4.8).
    >
    > There are 6 callers of PySequence_GetItem() in plpython, and none of
    > them checks the returned result before using it.  PySequence_GetItem()
    > can return NULL whenever an element cannot be fetched, so an object
    > that claims a length it cannot actually deliver is enough to crash the
    > backend.
    >
    > For example:
    >
    >   CREATE FUNCTION test() RETURNS int[] AS $$
    >   class C:
    >       def __len__(self):
    >           return 2
    >       def __getitem__(self, i):
    >           raise ValueError('boom')
    >   return C()
    >   $$ LANGUAGE plpython3u;
    >
    >   SELECT test();   -- crashes
    >
    >
    > The attached patch checks the result of PySequence_GetItem() in each
    > place and errors out if it is NULL.
    >
    
    Thanks for the patch and detailed repro.
    
    I applied the patch and it works well, changes too, LGTM.
    
    I think there's a similar problem on the mapping side that v1 doesn't
    cover. PLyMapping_ToJsonbValue and the hstore equivalent fetch items with
    PyMapping_Items() and PyList_GetItem() without checking for NULL, and a
    mapping whose items() raises still would crash the backend.
    
    Regards,
    Ayush
    
  3. Re: plpython: NULL pointer dereference on broken sequence objects

    Richard Guo <guofenglinux@gmail.com> — 2026-06-26T06:05:40Z

    On Thu, Jun 25, 2026 at 10:05 PM Ayush Tiwari
    <ayushtiwari.slg01@gmail.com> wrote:
    > I think there's a similar problem on the mapping side that v1 doesn't
    > cover. PLyMapping_ToJsonbValue and the hstore equivalent fetch items with
    > PyMapping_Items() and PyList_GetItem() without checking for NULL, and a
    > mapping whose items() raises still would crash the backend.
    
    Right.  Those functions have the same issue as PySequence_GetItem().
    The attached v2 patch fixes them all.
    
    - Richard
    
  4. Re: plpython: NULL pointer dereference on broken sequence objects

    Ayush Tiwari <ayushtiwari.slg01@gmail.com> — 2026-06-26T07:22:37Z

    Hi,
    
    On Fri, 26 Jun 2026 at 11:35, Richard Guo <guofenglinux@gmail.com> wrote:
    
    > On Thu, Jun 25, 2026 at 10:05 PM Ayush Tiwari
    > <ayushtiwari.slg01@gmail.com> wrote:
    > > I think there's a similar problem on the mapping side that v1 doesn't
    > > cover. PLyMapping_ToJsonbValue and the hstore equivalent fetch items with
    > > PyMapping_Items() and PyList_GetItem() without checking for NULL, and a
    > > mapping whose items() raises still would crash the backend.
    >
    > Right.  Those functions have the same issue as PySequence_GetItem().
    > The attached v2 patch fixes them all.
    >
    
    Thanks for the update! v2 LGTM.
    
    Regards,
    Ayush
    
  5. Re: plpython: NULL pointer dereference on broken sequence objects

    Richard Guo <guofenglinux@gmail.com> — 2026-06-29T03:01:16Z

    On Fri, Jun 26, 2026 at 4:22 PM Ayush Tiwari
    <ayushtiwari.slg01@gmail.com> wrote:
    > On Fri, 26 Jun 2026 at 11:35, Richard Guo <guofenglinux@gmail.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
    
    
    
    
  6. Re: plpython: NULL pointer dereference on broken sequence objects

    Ewan Young <kdbase.hack@gmail.com> — 2026-07-07T10:08:42Z

    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@gmail.com> wrote:
    >
    > On Fri, Jun 26, 2026 at 4:22 PM Ayush Tiwari
    > <ayushtiwari.slg01@gmail.com> wrote:
    > > On Fri, 26 Jun 2026 at 11:35, Richard Guo <guofenglinux@gmail.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