On Tue, Sep 22, 2026 at 10:10 PM Chao Li <[email protected]> wrote: > > > > > On Sep 23, 2026, at 12:19, Masahiko Sawada <[email protected]> wrote: > > > > On Tue, Sep 22, 2026 at 7:08 PM Chao Li <[email protected]> wrote: > >> > >> > >> > >>> On Sep 23, 2026, at 04:27, Masahiko Sawada <[email protected]> wrote: > >>> > >>> On Mon, Sep 21, 2026 at 10:00 PM Chao Li <[email protected]> wrote: > >>>> > >>>> > >>>> > >>>>> On Sep 22, 2026, at 03:51, Masahiko Sawada <[email protected]> > >>>>> 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
