Re: Adding a range check on the sequence index from the publisher.

On Tue, Sep 22, 2026 at 10:10 PM Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
>
>
>
> > On Sep 23, 2026, at 12:19, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> >
> > On Tue, Sep 22, 2026 at 7:08 PM Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
> >>
> >>
> >>
> >>> On Sep 23, 2026, at 04:27, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> >>>
> >>> On Mon, Sep 21, 2026 at 10:00 PM Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
> >>>>
> >>>>
> >>>>
> >>>>> On Sep 22, 2026, at 03:51, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> >>>>>
> >>>>> Hi all,
> >>>>> (CCing Amit as the committer of this feature)
> >>>>>
> >>>>> This was originally reported to pgsql-security by Anthropic OSS
> >>>>> program but the security team considered it as a non-vuln bug since
> >>>>> it's a v19-beta code, and I'm reporting here on behalf of them as it's
> >>>>> permitted now.
> >>>>>
> >>>>> The reported problem is in sequencesync.c; the sequence
> >>>>> synchronization worker uses an integer that came back from the
> >>>>> publisher as a list subscript without checking it, and then writes
> >>>>> through the resulting pointer.
> >>>>>
> >>>>> While it's not a problem in normal cases where the publisher is a
> >>>>> normal PostgreSQL, it could lead to out-of-bounds writes when the
> >>>>> publisher is a malicious server looking like a publisher.
> >>>>>
> >>>>> Other fields that we get through get_and_validate_seq_info() could
> >>>>> also get the wrong value but they just show the wrong values rather
> >>>>> than OOB writes. So I think we need a safeguard only for seqidx.
> >>>>>
> >>>>> I've attached the patch to fix it. Feedback is very welcome.
> >>>>>
> >>>>> Regards,
> >>>>>
> >>>>> --
> >>>>> Masahiko Sawada
> >>>>> Amazon Web Services: https://aws.amazon.com
> >>>>> <v1-0001-Add-a-range-check-on-the-sequence-index-from-the-.patch>
> >>>>
> >>>> If the concern here is a malicious publisher, does it also make sense to replace Assert(!isnull) with a runtime check and fail if seqidx is NULL?
> >>>
> >>> I don't think we need it from a security perspective. Even if a
> >>> malicious publisher returns NULL as seqidx, a garbage value is stored
> >>> to *seqidx and will fail the new range check.
> >>>
> >>
> >> If a malicious publisher returns NULL for seqidx, the resulting *seqidx will likely be 0. Since 0 passes the range check, the first sequence could be silently selected, which might be incorrect.
> >
> > Right.
> >
> >> On second thought, however, a malicious publisher could directly return a valid but incorrect seqidx, and we do not seem to have a way to protect against that. From this perspective, checking isnull would not help much.
> >
> > Agreed, and I think that is the important point. We have no way to
> > tell a malicious value from a buggy one, so validating the value
> > doesn't really make sense. A publisher reporting a wrong last_value is
> > indistinguishable from a publisher whose sequence really holds that
> > value, so it can change the sequence on the subscriber whatever we
> > check.
> >
> > I think what we need to fix here is narrower: the case where the
> > damage goes beyond the sequence being synchronized. The other columns
> > only lead to a wrong sequence value or a wrong report. seqidx is the
> > only one that becomes a list subscript, and so we write last_value
> > through a pointer taken from outside of the list.
>
> How about explaining that more explicitly in the comment? For example, the check prevents out-of-bounds access, but cannot protect against an incorrect index that is still within the valid range.

How about the following?

/*
* The publisher only echoes back an index that we put in the VALUES list,
* so this should always identify an entry of seqinfos. Check it anyway
* before using it as a list subscript, since list_nth() does not
* bounds-check on non-assert builds and we would then write the remote
* sequence state through a pointer fetched from beyond the list.
*
* This only keeps the subscript inside the list. An index that is wrong
* but still in range is not detected, and cannot be; the sequence it
* points at then receives another sequence's data. That is the same kind
* of damage as the publisher reporting a wrong value in any other column,
* and is likewise beyond what we can check.
*/

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com