| From: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
|---|---|
| To: | Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> |
| Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
| Subject: | Re: Adding a range check on the sequence index from the publisher. |
| Date: | 2026-09-23 02:08:10 |
| Message-ID: | EDFC20AF-1AC8-4ABC-BFEF-33A602BC5655@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
> 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.
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.
But from another perspective, an Assert is normally used for an internal invariant. Here, however, seqidx is received from external, so a runtime check seems more reasonable.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Khoa Nguyen | 2026-09-23 02:20:22 | Re: HASH INDEX builds seems confused |
| Previous Message | Xuneng Zhou | 2026-09-23 01:54:09 | Re: test: avoid redundant standby catchup in 049_wait_for_lsn |