Re: Adding a range check on the sequence index from the publisher. - Mailing list pgsql-hackers

From Chao Li
Subject Re: Adding a range check on the sequence index from the publisher.
Date
Msg-id EDFC20AF-1AC8-4ABC-BFEF-33A602BC5655@gmail.com
Whole thread
In response to Re: Adding a range check on the sequence index from the publisher.  (Masahiko Sawada <sawada.mshk@gmail.com>)
Responses Re: Adding a range check on the sequence index from the publisher.
List pgsql-hackers

> On Sep 23, 2026, at 04:27, Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> On Mon, Sep 21, 2026 at 10:00 PM Chao Li <li.evan.chao@gmail.com> wrote:
>>
>>
>>
>>> On Sep 22, 2026, at 03:51, Masahiko Sawada <sawada.mshk@gmail.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
checkand 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. 

On second thought, however, a malicious publisher could directly return a valid but incorrect seqidx, and we do not
seemto have a way to protect against that. From this perspective, checking isnull would not help much. 

But from another perspective, an Assert is normally used for an internal invariant. Here, however, seqidx is received
fromexternal, so a runtime check seems more reasonable. 

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/







pgsql-hackers by date:

Previous
From: Xuneng Zhou
Date:
Subject: Re: test: avoid redundant standby catchup in 049_wait_for_lsn
Next
From: Khoa Nguyen
Date:
Subject: Re: HASH INDEX builds seems confused